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

Reopening BlitzGateway returns False #413

Closed
barrettMCW opened this issue Jun 14, 2024 · 8 comments
Closed

Reopening BlitzGateway returns False #413

barrettMCW opened this issue Jun 14, 2024 · 8 comments

Comments

@barrettMCW
Copy link
Contributor

When I close the BlitzGateway then reopen it, it fails to connect, I think it's trying to reuse the client object which is attached to a closed session. I fixed it by doing:

...
self.conn.close(hard=False) # hard closure doesn't seem to effect the issue
self.conn._resetOmeroClient()

This is called in a thread, so there might be some extra weirdness involved in causing this bug.
Would there be any issues with adding the conn._resetOmeroClient method in conn.close?
I can provide the full code I'm using to cause this bug if you want to attempt to replicate this, it's a relatively small class.

@will-moore
Copy link
Member

Hi, thanks for the report.
There was recently a lot of discussion on BlitzGateway connection behaviour and some tweaks in #400, so it would be good to confirm whether this issue is affected by that?

And also check that your proposed changes doesn't break the behaviour e.g. with the tests at #400 (review)

Please feel free to open a PR with your proposed change and then we can continue the discussion there (since it will be tested by ci jobs etc).
cc @sbesson @jburel

@barrettMCW
Copy link
Contributor Author

The exception looks a little different now, but sadly still failing to reconnect:

DEBUG:omero.gateway:Connect attempt, sUuid=None, group=None, self.sUuid=None
DEBUG:omero.gateway:Ooops. no self._c
INFO:root:Refresh operation completed.
INFO:omero.gateway:closed connection (uuid=None)
Exception in thread Thread-6 (compile_response):
Traceback (most recent call last):
  File "/usr/lib/python3.10/threading.py", line 1016, in _bootstrap_inner
    self.run()
  File "/home/coder/.local/lib/python3.10/site-packages/ipykernel/ipkernel.py", line 766, in run_closure
    _threading_Thread_run(self)
  File "/usr/lib/python3.10/threading.py", line 953, in run
    self._target(*self._args, **self._kwargs)
  File "/tmp/ipykernel_3518/4272059447.py", line 41, in compile_response
Exception: Connection to OMERO server failed.

I'll get working on a PR

@sbesson
Copy link
Member

sbesson commented Jun 24, 2024

Thanks for the scenario @barrettMCW. Having reviewed the current implementation, probably the outstanding question is whether a BlitzGateway object is expected to reconnect at all after close has been invoked on it (either directly or via a context manager) or whether it should be treated as a resource.

Assuming the latter is the expectation, the BlitzGateway.clone() API might offer a possible solution as it allows to instantiate a new object and preserve its properties. In other terms, the following code should work:

from omero.gateway import BlitzGateway

with BlitzGateway(host=host,username=username, passwd=passwd) as conn:
    conn.connect()
    # do stuff
    new_conn = conn.clone()

conn.connect() # should return False
new_conn.connect() # should return True

@barrettMCW
Copy link
Contributor Author

barrettMCW commented Jun 24, 2024

Thanks for reviewing this @sbesson! The cloning method fixes my issues. From a pure semantic view, it makes sense to me that a gateway would open and close multiple times. But requiring a clone seems like a perfectly valid mechanism.
I think if we go with the clone method we should add an example to the omero-py docs. I'll put it on my board but I probably won't get to it today.
Up to y'all, either works good for me. Thanks again!

@sbesson
Copy link
Member

sbesson commented Jul 11, 2024

@will-moore @jburel what is your position on the reconnection scenario?

@will-moore
Copy link
Member

I don't have a strong opinion on whether we should support conn.connect() after connection is closed, mostly because I haven't come across use cases that need that. If those cases can be solved with conn.clone() then I'm happy to delay changes for now.

@jburel
Copy link
Member

jburel commented Jul 11, 2024

Most usage of the gateway in code or example assume that the services will be shut down when the gateway is closed.
Considering the recent work to fix issue with the constructor flexibility (see #400), i think the usage of clone is an acceptable alternative for the time being.
We should probably document that so it does not get lost

@barrettMCW
Copy link
Contributor Author

Works for me! I'll open a new issue for documenting the solution as described here. Thanks y'all!

@barrettMCW barrettMCW closed this as not planned Won't fix, can't repro, duplicate, stale Jul 11, 2024
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

No branches or pull requests

4 participants