search

ome / omero-py

Python project containing Ice remoting code for OMERO
https://www.openmicroscopy.org/omero
GNU General Public License v2.0
22 stars 32 forks source link

Chown gateway #195

Closed will-moore closed 4 years ago

will-moore commented 4 years ago

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

Tests added in https://github.com/ome/openmicroscopy/pull/6223

joshmoore commented 4 years ago

@will-moore : please see https://github.com/ome/omero-py/issues/153 for a possible leak with this usage.

will-moore commented 4 years ago

@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 commented 4 years ago

@will-moore : the block from https://github.com/ome/openmicroscopy/pull/6082#issuecomment-512740248

@@ -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 commented 4 years ago

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 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 commented 4 years ago

Probably needs the same fix as omero-web https://github.com/ome/omero-web/pull/152

will-moore commented 4 years ago

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 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', ) INTERNALERROR> assert not 'test/unit/tablestest/test_servants.py::TestTables::testTablePreInitialized' ```
joshmoore commented 4 years ago

Let's concentrate on https://github.com/ome/omero-py/pull/198 to try to fix all open PRs.

joshmoore commented 4 years ago

Re-opening with #198 in.

joshmoore commented 4 years ago

Green!

sbesson commented 4 years ago

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 commented 4 years ago

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 https://github.com/ome/omero-web/pull/149 but needs this to be in place first, as well as #196 from omero-py.

sbesson commented 4 years ago

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 commented 4 years ago

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 _?)...