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

Support Python 3 asyncio concurrency #194

Draft
wants to merge 3 commits into
base: master
Choose a base branch
from

Conversation

manics
Copy link
Member

@manics manics commented Mar 5, 2020

Adds support for creating an OMERO session and Ice services that can be used with asyncio.

  • 00707b7 creates an exact copy of createSession called createAsyncSession
  • a6583d0 modifies createAsyncSession to create an async session

Example:
https://gist.github.com/manics/22583d9404022f1b57df0e40f97b4cd3

@manics
Copy link
Member Author

manics commented Jun 2, 2020

Does anyone have opinions on how to support this usage? I initially implemented it as a separate python module, but the async version of createSession, createAsyncSession, is problematic to implement outside this module. createSession uses __private variables, so re-implementing this method in another class requires one of:

  • break the encapsulation and unmangle the private variables: e.g. self.__sf is referenced as self._BaseClient__sf, see for example https://github.com/manics/omero-asyncio/blob/4515a76a2ce9e7631b4bdb9ba3d2fb8829b97a65/omero_asyncio/omero_asyncio.py#L135-L276
  • duplicate all private variables and methods called by createSession: createAsyncSession has to call other class methods that also reference some __private variables, so if those private variables are duplicated in the subclass then all methods that reference them must also be duplicated
  • refactor createSession to support overriding: potentially dangerous given the complexity of the existing code

@joshmoore
Copy link
Member

I'm for (3) refactoring. Happy to help.

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

Successfully merging this pull request may close these issues.

2 participants