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

Assert connection decorator, see #289 #290

Closed
wants to merge 6 commits into from
Closed

Assert connection decorator, see #289 #290

wants to merge 6 commits into from

Conversation

glyg
Copy link
Contributor

@glyg glyg commented May 28, 2021

@will-moore suggested solving #289 with an assertConnected decorator

IDK how to set up a mock connection to test. I tested with my instance, but the current test wont pass

@will-moore
Copy link
Member

Since these are just unit tests, you probably don't want to do all the work needed to mock-up conn.getEventContext()

Instead, you could simply test the decorator itself (simple example below) and if it's also needed to test this for conn.getEventContext() etc then move that to the integration tests at https://github.com/ome/openmicroscopy/tree/develop/components/tools/OmeroPy/test/integration

    class MockGateway():
        _connected = False

        def connect(self):
            self._connected = True

    def foo(conn):
        pass

    wrapped_foo = assertConnected(foo)

    mock_conn = MockGateway()
    with pytest.raises(Ice.ConnectionLostException):
        wrapped_foo(mock_conn)
    
    mock_conn.connect()
    wrapped_foo(mock_conn)

@glyg
Copy link
Contributor Author

glyg commented May 31, 2021

I put the class def inside the test function as I saw there were other mocks in the file, not sure it is the right thing to do

@will-moore
Copy link
Member

Currently failing at assert conn.connect() since that returns None

@glyg
Copy link
Contributor Author

glyg commented May 31, 2021

sorry, forgot to run the test before the push :/

@snoopycrimecop
Copy link
Member

snoopycrimecop commented Jun 1, 2021

Conflicting PR. Removed from build OMERO-python-superbuild-push#712. See the console output for more details.
Possible conflicts:

--conflicts Conflict resolved in build OMERO-python-superbuild-push#713. See the console output for more details.

@will-moore
Copy link
Member

I've closed #276 which seems to be the PR responsible for the conflict above. Hopefully should be in the merge build now...

@joshmoore
Copy link
Member

@will-moore : did you give this a try?

@will-moore
Copy link
Member

Yes:

With this PR:

>>> from omero.gateway import BlitzGateway
>>> conn = BlitzGateway("user", "pass", host="host", port=4064)
>>> conn.getEventContext()
Traceback (most recent call last):
  File "<stdin>", line 1, in <module>
  File "/Users/wmoore/Desktop/PY/omero-py/target/omero/gateway/__init__.py", line 1519, in wrapped
    raise Ice.ConnectionLostException("Trying to call %s while not connected" % func.__name__)
Ice.ConnectionLostException: <exception str() failed>

Seems to be working, although Ice.ConnectionLostException: <exception str() failed> doesn't look quite right.

Without this PR:

>>> conn.getEventContext()
Traceback (most recent call last):
  File "<stdin>", line 1, in <module>
  File "/opt/anaconda3/envs/omeroweb/lib/python3.9/site-packages/omero/gateway/__init__.py", line 2322, in getEventContext
    if self._ctx is None:
AttributeError: '_BlitzGateway' object has no attribute '_ctx'

@glyg
Copy link
Contributor Author

glyg commented Jul 12, 2021

raise Ice.ConnectionLostException("Trying to call %s while not connected" % func.__name__)
Ice.ConnectionLostException: <exception str() failed>

Uh? Maybe using a format string would solve the problem? Trying that now

@will-moore
Copy link
Member

I'm afraid that doesn't make a lot of difference for me, but don't know why not!

>>> conn.getEventContext()
Traceback (most recent call last):
  File "<stdin>", line 1, in <module>
  File "/Users/wmoore/Desktop/PY/omero-py/target/omero/gateway/__init__.py", line 1519, in wrapped
    raise Ice.ConnectionLostException(f"Trying to call {func.__name__} while not connected")
Ice.ConnectionLostException: <exception str() failed>

@glyg
Copy link
Contributor Author

glyg commented Jul 12, 2021

I tested generating the same kind of error message in a generic decorator and did not have the problem (with a ValueError). Maybe something strange going on with IceConnectionLostException? I'm trying without fomatting now

This pull request was closed.
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.

4 participants