#1761 closed task (fixed)
refactoring to avoid large modules
Reported by: | Antoine Martin | Owned by: | Antoine Martin |
---|---|---|---|
Priority: | major | Milestone: | 2.3 |
Component: | core | Version: | 2.2.x |
Keywords: | Cc: |
Description
As of r18260, we have some modules that are just too big.
$ find xpra/ -type f -name "*.py" -exec bash -c 'cat {}|wc -l|xargs echo -n;echo " {}"' \; | sort -n | tail -n 40 589 xpra/client/window_backing_base.py 602 xpra/x11/desktop_server.py 678 xpra/platform/win32/lsa_logon_lib.py 714 xpra/x11/gtk2/models/base.py 720 xpra/x11/gtk2/models/core.py 733 xpra/client/client_window_base.py 733 xpra/x11/xkbhelper.py 736 xpra/os_util.py 747 xpra/platform/win32/wndproc_events.py 770 xpra/x11/gtk2/models/window.py 773 xpra/net/file_transfer.py 798 xpra/util.py 834 xpra/platform/darwin/gui.py 890 xpra/x11/x11_server_core.py 895 xpra/sound/gstreamer_util.py 919 xpra/clipboard/clipboard_base.py 946 xpra/server/proxy/proxy_instance_process.py 950 xpra/platform/xposix/gui.py 1014 xpra/client/gtk_base/client_launcher.py 1031 xpra/client/client_base.py 1068 xpra/scripts/server.py 1100 xpra/client/gl/gl_window_backing_base.py 1100 xpra/net/protocol.py 1103 xpra/gtk_common/gtk_util.py 1173 xpra/client/gtk_base/session_info.py 1208 xpra/platform/win32/gui.py 1241 xpra/x11/server.py 1307 xpra/client/gtk_base/gtk_client_base.py 1379 xpra/scripts/config.py 1474 xpra/client/gtk_base/gtk_tray_menu_base.py 1528 xpra/client/gtk_base/gtk_client_window_base.py 1780 xpra/server/server_core.py 1934 xpra/server/window/window_video_source.py 2088 xpra/net/mdns/pybonjour.py 2276 xpra/server/window/window_source.py 2632 xpra/server/source.py 3402 xpra/scripts/main.py 3794 xpra/server/server_base.py 3904 xpra/client/ui_client_base.py 4717 xpra/platform/win32/constants.py
Some of those are fine: constants or external modules (ie: pybonjour), others deserve to be refactored.
This ticket is also in preparation for #1700.
Change History (7)
comment:2 Changed 4 years ago by
Status: | new → assigned |
---|
Lots more gradual changes to ui client, eventually splitting the code into mixins:
- r18512: clipboard mixin
- r18514: notification mixin
- r18516: window mixin
- r18517: move input devices into window mixin
- r18519: move systray forwarding into window mixin
- r18521: mmap mixin
- r18522: remote logging mixin
- r18525: display / scaling / desktop mixin
- r18526: move all mixins to a new "mixins" submodule
- r18529: network: ping / info request mixin
- r18530: encodings mixin
- r18531: add stub mixin base class
- r18532: xpra tray mixin
- r18534: unit tests for mmap and network-state mixins
- r18535: unit tests for remote logging mixin
Minor fixups and tweaks: r18513, r18515, r18518, r18520, r18523, r18524, r18527, r18527, r18528, r18533, r18533
Not yet dealt with:
init_ui
is still a bit messy- more consistent namespace for mixin variables, try to distinguish: settings (command line, env vars), server settings (from hello or update packets), state (focus, buttons, etc)
- some timers can now be tracked and cancelled more easily (ie: ping timers)
The ui client is now much smaller (~620 lines) and all the mixins file sizes are now much more reasonable:
$ wc -l xpra/client/ui_client_base.py xpra/client/mixins/* 620 xpra/client/ui_client_base.py 562 xpra/client/mixins/audio_client.py 180 xpra/client/mixins/clipboard_client.py 517 xpra/client/mixins/display_client.py 293 xpra/client/mixins/encodings.py 4 xpra/client/mixins/__init__.py 123 xpra/client/mixins/mmap_client.py 189 xpra/client/mixins/network_state.py 155 xpra/client/mixins/notification_client.py 94 xpra/client/mixins/remote_logging.py 119 xpra/client/mixins/rpc_client.py 36 xpra/client/mixins/stub_client_mixin.py 125 xpra/client/mixins/tray_client.py 310 xpra/client/mixins/webcam_forwarder.py 1296 xpra/client/mixins/window_client.py 4623 total
The line count has actually increased, but the code is in much better shape, much more readable and maintainable.
Next: the server.
comment:3 Changed 4 years ago by
Server base class split into mixins:
- r18539: notifications
- r18540: webcam
- r18541: clipboard
- r18543: audio
- r18544: file-transfers + printing
- r18545: mmap
- r18546: input devices
- r18548: start commands
- r18549: dbus rpc
- r18550: encodings
- r18551: remote logging
- r18554: network state
- r18555: display management
- r18556: window management
Minor fixups in r18542, r18547, r18552, r18553, r18557
Still TODO for server base class:
do_cleanup
vscleanup
: make it consistentsetup
should not need optsparse_hello_ui_clipboard
and more pollution in ui hello parsing- clipboard is broken? (client side problem?)
- client_reset in clipboard helper
- init_sockets printing
Which gives us:
$ wc -l xpra/server/server_base.py xpra/server/mixins/* 973 xpra/server/server_base.py 297 xpra/server/mixins/audio_server.py 283 xpra/server/mixins/child_command_server.py 189 xpra/server/mixins/clipboard_server.py 102 xpra/server/mixins/dbusrpc_server.py 251 xpra/server/mixins/display_manager.py 212 xpra/server/mixins/encoding_server.py 266 xpra/server/mixins/fileprint_server.py 4 xpra/server/mixins/__init__.py 401 xpra/server/mixins/input_server.py 53 xpra/server/mixins/logging_server.py 40 xpra/server/mixins/mmap_server.py 156 xpra/server/mixins/networkstate_server.py 143 xpra/server/mixins/notification_forwarder.py 543 xpra/server/mixins/server_base_controlcommands.py 31 xpra/server/mixins/stub_server_mixin.py 247 xpra/server/mixins/webcam_server.py 305 xpra/server/mixins/window_server.py 4496 total
We should write a lot more unit tests for those mixins: #1773
comment:4 Changed 4 years ago by
Now done for "Server Source",
- r18560: audio mixin and
ServerSource
renamed toClientConnection
- r18561: mmap mixin
- r18562: clipboard mixin
- r18563: file-transfers + printing
- r18564: network-state (pings)
- r18565: client info (build info, version, proxy details, etc)
- r18566 + r18567: rename mixins
- r18570: notifications refactoring
- r18571: dbus mixin
- r18572: window management
- r18573: encodings
- r18574: idle detection
- r18575: input handling (keyboard, pointer)
- r18576: av-sync
- r18577: client-display state
- r18569: expose the list of packet handlers via
xpra info
Minor fixups in r18568.
The result:
$ wc -l xpra/server/source/* 479 xpra/server/source/audio_mixin.py 61 xpra/server/source/avsync_mixin.py 449 xpra/server/source/client_connection.py 88 xpra/server/source/clientdisplay_mixin.py 127 xpra/server/source/clientinfo_mixin.py 105 xpra/server/source/clipboard_connection.py 36 xpra/server/source/dbus_mixin.py 540 xpra/server/source/encodings_mixin.py 146 xpra/server/source/fileprint_mixin.py 93 xpra/server/source/idle_mixin.py 4 xpra/server/source/__init__.py 101 xpra/server/source/input_mixin.py 114 xpra/server/source/mmap_connection.py 105 xpra/server/source/networkstate_mixin.py 235 xpra/server/source/source_stats.py 28 xpra/server/source/stub_source_mixin.py 592 xpra/server/source/windows_mixin.py 3303 total
New TODO:
- unit tests
- move webcam server code there, webcam forwarding should be per-client (and think about handling more than one per client?)
- mmap still referenced from other mixins: encodings and windows
- suspend + resume code should also suspend audio?
- networkstate: pings require statistics object
- don't register packet handlers if feature is off
- minimize import deps on mixin load (ie: done for window source in r18578)
- modifier_client_keycodes still ugly
comment:5 Changed 4 years ago by
Lots more updates:
- r18656 disable more features and their packet handlers if we find that we cannot load them or that they are disabled by the configuration
- r18654 import as needed
- r18653 docstrings
- r18652 generalize init_sockets so we can move some domain specific checks to the fileprint mixin
- r18651 generalize protocol cleanup callbacks, move clipboard cleanup to this callback
- r18650 move system-tray to x11 server, don't pass options to setup method
- r18624 move server info bits to a mixin, move remaining encoding attributes to the encodings mixin
- r18623 move file+print to a mixin, only init aliases after we have initialized the packet handlers
- r18598 gtk cleanup, use MRO, fix clipboard, consistency, etc
- r18596 generically call
parse_client_caps
on all superclasses - r18595 try harder to split state variables from configuration options
- r18594 add av-sync unit test, minor cleanups
- r18593 webcam: tests, use libyuv, etc
- r18592 move idle handling code to client-connection, generalize init_state source mixin method
- r18591 rename server mixin test, add source mixin test
- r18588 move webcam handling to a source mixin, so we can have one or more virtual webcam per connection
- r18584 split caps into smaller methods, use namespace and only flatten the dicts as late as possible
- r18583 move "scaling-control" option to encodings where it belongs, expose more info from display manager
- r18579 move window-icon code to a mixin
- r18578 reduce imports at module load time: import window source when needed instead
- r18577 move client-display code to a mixin
Minor fixups: r18620, r18609, r18606, r18600, r18599, r18597, r18590, r18585, r18581, r18580
comment:6 Changed 4 years ago by
Resolution: | → fixed |
---|---|
Status: | assigned → closed |
comment:7 Changed 16 months ago by
this ticket has been moved to: https://github.com/Xpra-org/xpra/issues/1761
First round:
ui client:
Result:
xpra/scripts/main.py
: 2229xpra/server/server_base.py
: 3341 linesxpra/client/ui_client_base.py
: 3009 linesStill big, but more readable and manageable.
Some of those changes can also help us write better unit tests.(#847)