martius-lab / cluster_utils

https://cluster-utils.readthedocs.io/stable/
Other
12 stars 0 forks source link

Move client functions to sub-package "client" #106

Closed luator closed 5 months ago

luator commented 5 months ago

Move the functions that are supposed to be used by the client to a new subpackage client (as discussed in #45).

I put the public functions (and some private helpers) directly into the __init__.py. It feels a bit weird to me to have actual implementations there, but I think it's actually rather common practice. The functions for communication with the server are moved to client.server_communication.

The start_time variable is moved to the submission_state module. I think it fits there well and the use of a function attribute for this has always bothered me (and mypy).

Since everything is still imported in cluster_utils/__init__.py, these changes should not affect any user code.

luator commented 5 months ago

There are actually still some imports of cluster_utils modules in cluster_utils.client, I think we should try to get rid of them, if possible.

luator commented 5 months ago

I just looked into it. There was one function which wasn't used anywhere else, so I moved that as well. The rest is also used in other modules, so having a full separation isn't easily possible.

mseitzer commented 5 months ago

I just looked into it. There was one function which wasn't used anywhere else, so I moved that as well. The rest is also used in other modules, so having a full separation isn't easily possible.

Yes, full separation is not possible, as client and server need to have shared message types to send.

What I had in mind in #83 is to have a (fully separate) package that contains the client API and shared definitions, and then having the server package depend on that package. Another option is to have a "core" package, and then having two client and server packages that both depend on the "core" package. Although this seems like overkill.