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

Chown gateway #195

Merged
merged 5 commits into from
Jun 3, 2020
Merged

Chown gateway #195

merged 5 commits into from
Jun 3, 2020

Conversation

will-moore
Copy link
Member

@will-moore will-moore commented Mar 6, 2020

Started to add Chown functionality to Blitz Gateway to allow for support in webclient.

Tests added in ome/openmicroscopy#6223

@joshmoore
Copy link
Member

@will-moore : please see #153 for a possible leak with this usage.

@will-moore
Copy link
Member Author

@josh Is there a workaround for the leak?
We do the same in conn.deleteObjects() so I guess we need to apply the same fix there too.

@joshmoore
Copy link
Member

@will-moore : the block from ome/openmicroscopy#6082 (comment)

@@ -339,6 +339,7 @@ def forgotten_password(request, **kwargs):
                     try:
                         conn._waitOnCmd(handle)
                     finally:
+                        # This likely leaks a callback servant
                         handle.close()

should be:

                     try:
                         cb = conn._waitOnCmd(handle)
                     finally:
                         cb.close(True)

i.e. if you are using a callback, close via the callback and not via the handle with the issue being that we don't generally consider _waitOnCmd to be using a callback, but it is internally.

@will-moore
Copy link
Member Author

Travis failing with tests

INTERNALERROR>     self.writer = self._tw
INTERNALERROR> AttributeError: can't set attribute
Full error
py36 run-test: commands[2] | pytest -n4 -m 'not broken' --reruns 5 -rf test
INTERNALERROR> Traceback (most recent call last):
INTERNALERROR>   File "/home/travis/build/ome/omero-py/.tox/py36/lib/python3.6/site-packages/_pytest/main.py", line 187, in wrap_session
INTERNALERROR>     config._do_configure()
INTERNALERROR>   File "/home/travis/build/ome/omero-py/.tox/py36/lib/python3.6/site-packages/_pytest/config/__init__.py", line 820, in _do_configure
INTERNALERROR>     self.hook.pytest_configure.call_historic(kwargs=dict(config=self))
INTERNALERROR>   File "/home/travis/build/ome/omero-py/.tox/py36/lib/python3.6/site-packages/pluggy/hooks.py", line 308, in call_historic
INTERNALERROR>     res = self._hookexec(self, self.get_hookimpls(), kwargs)
INTERNALERROR>   File "/home/travis/build/ome/omero-py/.tox/py36/lib/python3.6/site-packages/pluggy/manager.py", line 93, in _hookexec
INTERNALERROR>     return self._inner_hookexec(hook, methods, kwargs)
INTERNALERROR>   File "/home/travis/build/ome/omero-py/.tox/py36/lib/python3.6/site-packages/pluggy/manager.py", line 87, in <lambda>
INTERNALERROR>     firstresult=hook.spec.opts.get("firstresult") if hook.spec else False,
INTERNALERROR>   File "/home/travis/build/ome/omero-py/.tox/py36/lib/python3.6/site-packages/pluggy/callers.py", line 208, in _multicall
INTERNALERROR>     return outcome.get_result()
INTERNALERROR>   File "/home/travis/build/ome/omero-py/.tox/py36/lib/python3.6/site-packages/pluggy/callers.py", line 80, in get_result
INTERNALERROR>     raise ex[1].with_traceback(ex[2])
INTERNALERROR>   File "/home/travis/build/ome/omero-py/.tox/py36/lib/python3.6/site-packages/pluggy/callers.py", line 187, in _multicall
INTERNALERROR>     res = hook_impl.function(*args)
INTERNALERROR>   File "/home/travis/build/ome/omero-py/.tox/py36/lib/python3.6/site-packages/pytest_sugar.py", line 176, in pytest_configure
INTERNALERROR>     sugar_reporter = SugarTerminalReporter(standard_reporter)
INTERNALERROR>   File "/home/travis/build/ome/omero-py/.tox/py36/lib/python3.6/site-packages/pytest_sugar.py", line 214, in __init__
INTERNALERROR>     self.writer = self._tw
INTERNALERROR> AttributeError: can't set attribute
ERROR: InvocationError for command /home/travis/build/ome/omero-py/.tox/py36/bin/pytest -n4 -m 'not broken' --reruns 5 -rf test (exited with code 3)

@manics
Copy link
Member

manics commented Mar 17, 2020

Probably needs the same fix as omero-web ome/omero-web#152

@will-moore
Copy link
Member Author

Thanks @manics - that helped get a bit further.
Now tests are failing for python 3.6 only with:

INTERNALERROR> AssertionError: ('test/unit/tablestest/test_servants.py::TestTables::testTablePreInitialized', <WorkerController gw0>)
INTERNALERROR> assert not 'test/unit/tablestest/test_servants.py::TestTables::testTablePreInitialized'
Full error
........................................................................ [ 92%]
........ss.sss.s.ss.ss.sssssssssssss.sssssss............................ [ 97%]
...........x....R.INTERNALERROR> Traceback (most recent call last):
INTERNALERROR>   File "/home/travis/build/ome/omero-py/.tox/py36/lib/python3.6/site-packages/_pytest/main.py", line 191, in wrap_session
INTERNALERROR>     session.exitstatus = doit(config, session) or 0
INTERNALERROR>   File "/home/travis/build/ome/omero-py/.tox/py36/lib/python3.6/site-packages/_pytest/main.py", line 247, in _main
INTERNALERROR>     config.hook.pytest_runtestloop(session=session)
INTERNALERROR>   File "/home/travis/build/ome/omero-py/.tox/py36/lib/python3.6/site-packages/pluggy/hooks.py", line 286, in __call__
INTERNALERROR>     return self._hookexec(self, self.get_hookimpls(), kwargs)
INTERNALERROR>   File "/home/travis/build/ome/omero-py/.tox/py36/lib/python3.6/site-packages/pluggy/manager.py", line 93, in _hookexec
INTERNALERROR>     return self._inner_hookexec(hook, methods, kwargs)
INTERNALERROR>   File "/home/travis/build/ome/omero-py/.tox/py36/lib/python3.6/site-packages/pluggy/manager.py", line 87, in <lambda>
INTERNALERROR>     firstresult=hook.spec.opts.get("firstresult") if hook.spec else False,
INTERNALERROR>   File "/home/travis/build/ome/omero-py/.tox/py36/lib/python3.6/site-packages/pluggy/callers.py", line 208, in _multicall
INTERNALERROR>     return outcome.get_result()
INTERNALERROR>   File "/home/travis/build/ome/omero-py/.tox/py36/lib/python3.6/site-packages/pluggy/callers.py", line 80, in get_result
INTERNALERROR>     raise ex[1].with_traceback(ex[2])
INTERNALERROR>   File "/home/travis/build/ome/omero-py/.tox/py36/lib/python3.6/site-packages/pluggy/callers.py", line 187, in _multicall
INTERNALERROR>     res = hook_impl.function(*args)
INTERNALERROR>   File "/home/travis/build/ome/omero-py/.tox/py36/lib/python3.6/site-packages/xdist/dsession.py", line 112, in pytest_runtestloop
INTERNALERROR>     self.loop_once()
INTERNALERROR>   File "/home/travis/build/ome/omero-py/.tox/py36/lib/python3.6/site-packages/xdist/dsession.py", line 135, in loop_once
INTERNALERROR>     call(**kwargs)
INTERNALERROR>   File "/home/travis/build/ome/omero-py/.tox/py36/lib/python3.6/site-packages/xdist/dsession.py", line 177, in worker_workerfinished
INTERNALERROR>     assert not crashitem, (crashitem, node)
INTERNALERROR> AssertionError: ('test/unit/tablestest/test_servants.py::TestTables::testTablePreInitialized', <WorkerController gw0>)
INTERNALERROR> assert not 'test/unit/tablestest/test_servants.py::TestTables::testTablePreInitialized'

@joshmoore
Copy link
Member

Let's concentrate on #198 to try to fix all open PRs.

@joshmoore
Copy link
Member

Re-opening with #198 in.

@joshmoore joshmoore closed this Apr 3, 2020
@joshmoore joshmoore reopened this Apr 3, 2020
@joshmoore
Copy link
Member

Green!

@joshmoore joshmoore merged commit cc791c2 into ome:master Jun 3, 2020
@sbesson sbesson added this to the 5.6.3 milestone Jun 9, 2020
@sbesson
Copy link
Member

sbesson commented Jun 9, 2020

As I am looking into cutting a patch release of omero-py with all included fixes (see https://github.com/ome/omero-py/milestone/4?closed=1), this PR contains the only change that worries me as it adds a new API to the omero.gateway and the discussion suggests this is still in-progress work.

@will-moore how confident do you feel about making this a public gateway API and supporting it? Additionally, is anything in our own stack consuming it yet?
One potential solution would be to mark this API as internal using the leading underscore convention (https://www.python.org/dev/peps/pep-0008/#public-and-internal-interfaces) i.e. def _chownObjects, release omero-py 5.6.3 and start consuming this API until we are happy to declare it as public API?

@will-moore
Copy link
Member Author

I'm pretty happy with releasing this.
It's a thin wrapper around the underlying API and we have tests.
The usage of it in our own stack is in progress at ome/omero-web#149
but needs this to be in place first, as well as #196 from omero-py.

@sbesson
Copy link
Member

sbesson commented Jun 9, 2020

Thanks. I think options are:

The first option is my least favorite one as we effectively block a component release for internal reasons while there are fixes that many users could consume immediately. I am equally fine with options 2 and 3.

@will-moore
Copy link
Member Author

I don't have a strong preference between those options.
Don't know how long #196 will take. No fixes requested yet...
Happy to open a PR to add a 5.7.0 changelog or make chown "internal" (by just prefixing with _?)...

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.

5 participants