Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

type cleanups and refactoring #3930

Closed
totaam opened this issue Jul 21, 2023 · 6 comments
Closed

type cleanups and refactoring #3930

totaam opened this issue Jul 21, 2023 · 6 comments
Labels
enhancement New feature or request

Comments

@totaam
Copy link
Collaborator

totaam commented Jul 21, 2023

Mostly flagged by #3927, related to #3592.

A lot of the types from the typing module are now deprecated, see PEP 585:
https://docs.python.org/3/library/typing.html#typing.Tuple

This requires Python 3.9 (not a big problem for v6?)

  • get_info server method signature mess:
xpra/platform/darwin/shadow_server.py:231: error: Signature of "get_info" incompatible with supertype "ServerBase"  [override]
xpra/platform/darwin/shadow_server.py:231: note:      Superclass:
xpra/platform/darwin/shadow_server.py:231: note:          def get_info(self, proto: Any = ..., client_uuids: Any = ...) -> dict[str, Any]
xpra/platform/darwin/shadow_server.py:231: note:      Subclass:
xpra/platform/darwin/shadow_server.py:231: note:          def get_info(self, proto: Any, *_args: Any) -> dict[str, Any]
  • stricter packet parsing at network boundary, ie:
    xpra/x11/server.py:916: error: Cannot determine type of "wid" [has-type]
    Could this be introspected and changed to a positional packet function with default args?
  • "client-window-base" missing attributes"
    xpra/client/gui/client_window_base.py:414: error: "ClientWindowBase" has no attribute "set_modal" [attr-defined]
  • mixins cross domain dependencies:
    xpra/server/mixins/audio.py:216: error: "AudioServer" has no attribute "add_process" [attr-defined]
  • SocketProtocol missing attributes: authenticators, encryption, ..
  • XpraConfig class does not have any "real" attributes: convert to introspected hints?
  • _id_to_window, readonly, ui_driver and friends
  • Return a value of type "tuple[bool, bool]" instead of "NoneType" or update function "accept_data" type hint
  • brain-overload: 164 functions have a cognitive complexity above 30 in sonarqube
  • 41 FIXMEs and 38 TODOs
  • 9 functions have too many arguments
    etc
@totaam totaam added the enhancement New feature or request label Jul 21, 2023
@zatricky
Copy link
Contributor

zatricky commented Jul 24, 2023

This seems also to be a problem for focal (Ubuntu 20.04 - corrected) as the latest version of xpra now requires python3.10+ due to using TypeAlias whereas focal ships with python3.8.10. I could be wrong about the exact versions needed/etc - here's the error at least:

2023-07-24 14:02:51,740 server error processing new connection from Protocol(socket socket:/run/user/<userid>/xpra/80/socket): cannot import name 'TypeAlias' from 'typing' (/usr/lib/python3.8/typing.py)
Traceback (most recent call last):
  File "/usr/lib/python3/dist-packages/xpra/server/server_core.py", line 2245, in call_hello_oked
    self.hello_oked(proto, c, auth_caps)
  File "/usr/lib/python3/dist-packages/xpra/server/server_base.py", line 395, in hello_oked
    cc_class = self.get_client_connection_class(c)
  File "/usr/lib/python3/dist-packages/xpra/server/server_base.py", line 420, in get_client_connection_class
    return get_client_connection_class(caps)
  File "/usr/lib/python3/dist-packages/xpra/server/source/client_connection_factory.py", line 83, in get_client_connection_class
    CC_BASES = get_needed_based_classes(caps)
  File "/usr/lib/python3/dist-packages/xpra/server/source/client_connection_factory.py", line 71, in get_needed_based_classes
    from xpra.server.source.client_connection import ClientConnection
  File "/usr/lib/python3/dist-packages/xpra/server/source/client_connection.py", line 10, in <module>
    from typing import Dict, Any, Optional, Tuple, Callable, Union, TypeAlias
ImportError: cannot import name 'TypeAlias' from 'typing' (/usr/lib/python3.8/typing.py)
2023-07-24 14:02:51,741 Disconnecting client /run/user/<userid>/xpra/80/socket:
2023-07-24 14:02:51,741  ConnectionMessage.CONNECTION_ERROR (error accepting new connection)

totaam added a commit that referenced this issue Jul 24, 2023
we have to import from 'typing_extensions' and add the package dependency..
@totaam
Copy link
Collaborator Author

totaam commented Jul 24, 2023

@zatricky thanks for pointing that out, the workaround in d37fecb should work (not tested yet).
The requirement for Python 3.6+ in v5 is unchanged.


Edit:

This seems also to be a problem for focal (Ubuntu 22.04)

FYI: focal is 20.04.

totaam added a commit that referenced this issue Jul 24, 2023
@zatricky
Copy link
Contributor

Oops! Yes, 20.04 ... :-|

@totaam
Copy link
Collaborator Author

totaam commented Jul 28, 2023

With the switch to Python 3.10+, we can also take advantage of:

totaam added a commit that referenced this issue Jul 28, 2023
totaam added a commit that referenced this issue Jul 28, 2023
totaam added a commit that referenced this issue Jul 28, 2023
totaam added a commit that referenced this issue Jul 28, 2023
totaam added a commit that referenced this issue Jul 28, 2023
@totaam
Copy link
Collaborator Author

totaam commented Jul 29, 2023

Remaining items that might be of interest:

totaam added a commit that referenced this issue Jul 30, 2023
totaam added a commit that referenced this issue Jul 30, 2023
totaam added a commit that referenced this issue Aug 8, 2023
totaam added a commit that referenced this issue Aug 8, 2023
totaam added a commit that referenced this issue Sep 1, 2023
totaam added a commit that referenced this issue Sep 2, 2023
totaam added a commit that referenced this issue Sep 2, 2023
totaam added a commit that referenced this issue Sep 2, 2023
@totaam totaam closed this as completed Sep 5, 2023
totaam added a commit that referenced this issue Sep 5, 2023
totaam added a commit that referenced this issue Sep 5, 2023
totaam added a commit that referenced this issue Sep 5, 2023
@totaam
Copy link
Collaborator Author

totaam commented Sep 26, 2023

Regarding the switch to monotonic_ns, here is the type of bug we don't want to re-introduce: 5768677 / #2038

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
None yet
Development

No branches or pull requests

2 participants