On 12/01/2018 12:56 AM, Sebastian Wagner wrote: > > > Am 30.11.18 um 14:34 schrieb Sage Weil: >> On Fri, 30 Nov 2018, Sebastian Wagner wrote: >>> All, >>> >>> looks like the architecture of the mgr supporting independent modules >>> was quite successful :-). >>> >>> In recent times, I'm seeing an increasing amount of coped copied from >>> other mgr modules. This has the obvious downside that a fix in one >>> module leaves other modules unfixed. Like for example PR #25337. >>> >>> Running `pylint --disable=all --enable=duplicate-code *` within >>> pybind/mgr reveals, in my opinion, too many duplicates. One also have to >>> count the places where code was semantically copied (or inspired by). >>> >>> One obvious reason is that the mgr modules are prone to replicating >>> code, as each module is build by many distinct teams for a specific and >>> independent use case. Also, testing other affected modules can be quite >>> challenging. >>> >>> Therefor, I'd like to raise awareness. Maybe, we can reduce copies in >>> the future. >> >> My main question is what our strategy should be. Where should shared code >> live? Should we transplant it all into mgr_module.py, or into util.py or >> similar in the pybind/mgr directory? > > pybind/mgr/util.py sounds good. To me, mgr_module.py sounds like a > Python module meant to contain the API to C++. Agreed. >> >> Current example: I'm using format_bytes() from the status module from the >> pg_autoscaler module via remote(), which is probably not ideal! > > Perfect example! We have a very similar function in > /src/ceph-volume/ceph_volume/util/disk.py called human_readable_size. We > don't have any common place for shared Python code right now. Same for the REST client piece in the dashboard; it'd be nice to refactor this out into a utility module somewhere so it could be reused by the DeepSea orchestrator module (and anything else that might need to speak REST in future). Regards, Tim -- Tim Serong Senior Clustering Engineer SUSE tserong@xxxxxxxx
