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

Deprecate duplicate plugin #249

Merged
merged 4 commits into from Sep 14, 2020
Merged

Conversation

sbesson
Copy link
Member

@sbesson sbesson commented Sep 10, 2020

As work has restarted on the decoupled https://github.com/ome/omero-cli-duplicate/ plugin, this formally deprecates the legacy code in omero-py using the similar strategy as #124. The plugin can then safely be dropped in omero-py 6.

This PR should be tested in two environment

  • omero-py alone
python3 -m venv venv
venv/bin/pip install git+git://github.com/snoopycrimecop/omero-py@merge_ci#egg=omero-py
venv/bin/omero duplicate -h # should fail
OMERO_DEV_PLUGINS=true venv/bin/omero duplicate -h # should display the help
OMERO_DEV_PLUGINS=true venv/bin/omero duplicate Dataset:50 # should display a deprecation warning
  • omero-py with omero-cli-duplicate plugin installed
python3 -m venv venv
venv/bin/pip install git+git://github.com/snoopycrimecop/omero-py@merge_ci#egg=omero-py
venv/bin/pip install git+git://github.com/snoopycrimecop/omero-cli-duplicate@merge_ci#egg=omero-cli-duplicate
venv/bin/omero duplicate -h # should display the help message
venv/bin/omero duplicate Dataset:50 # should duplicate without warnings

@sbesson
Copy link
Member Author

sbesson commented Sep 10, 2020

In addition to the manual testing above, I suspect this deprecation should allow https://merge-ci.openmicroscopy.org/jenkins/job/OMERO-test-integration/ to properly install and test the recent omero-cli-duplicate improvements

@will-moore
Copy link
Member

Testing omero-py alone I don't see a deprecation warning with:

$ OMERO_DEV_PLUGINS=true venv/bin/omero duplicate Dataset:232
Using session for will@localhost:4064. Idle timeout: 10 min. Current group: Lab Data
omero.cmd.Duplicate Dataset:232 ok

@sbesson
Copy link
Member Author

sbesson commented Sep 11, 2020

Ah. My changes might only apply in the case of a detailed report. Does $ OMERO_DEV_PLUGINS=true venv/bin/omero duplicate Dataset:232 --report display the warning?

@will-moore
Copy link
Member

Yes:

$ OMERO_DEV_PLUGINS=true venv/bin/omero duplicate Dataset:232 --report
Using session for will@localhost:4064. Idle timeout: 10 min. Current group: Lab Data
omero.cmd.Duplicate Dataset:232 ok
Steps: 8
Elapsed time: 0.038 secs.
Flags: []
This plugin is deprecated as of OMERO 5.8.0. Use the plugin available from https://pypi.org/project/omero-cli-duplicate/ instead.
Duplicates
  Dataset:1155

@will-moore
Copy link
Member

With the plugin installed, duplicate works without warning message. 👍

@sbesson
Copy link
Member Author

sbesson commented Sep 11, 2020

Cheers. Let me push another commit so that the warning message is displays on every invocation of the command

@will-moore
Copy link
Member

Works for me now, testing locally...

$ OMERO_DEV_PLUGINS=true venv/bin/omero duplicate Dataset:232
This plugin is deprecated as of OMERO.py 5.8.0. Use the plugin available from https://pypi.org/project/omero-cli-duplicate/ instead.
Previous session expired for will on localhost:4064
Server: [localhost:4064] 
Username: [will]
Password:
Created session for will@localhost:4064. Idle timeout: 10 min. Current group: Lab Data
omero.cmd.Duplicate Dataset:232 ok

@sbesson sbesson merged commit b7197b9 into ome:master Sep 14, 2020
@mtbc
Copy link
Member

mtbc commented Sep 16, 2020

OMERO-test-integration is failing, maybe we need to decide about the pip install omero-cli-duplicate for devspaces.

@sbesson
Copy link
Member Author

sbesson commented Sep 16, 2020

Probably largely depends on where we decide to run the integration test for the new omero-cli-duplicate functionalities:

  • if the tests are in ome/openmicroscopy, then OMERO-test-integration should install omero-cli-duplicate and we can port the change made in merge-ci
  • if the tests are in omero-cli-duplicate using omero-test-infra, then the upstream ome/openmicroscopy could be adjusted to either test the legacy plugin or removed

@mtbc
Copy link
Member

mtbc commented Sep 16, 2020

The for-the-meantime-at-least decision seemed to be to have them in ome/openmicroscopy.

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.

None yet

3 participants