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

CLI download looks for resources in all user groups (fix #12146). #2383

Merged
merged 7 commits into from Apr 30, 2014

Conversation

bpindelski
Copy link

This PR updates the logic used by bin\omero download. Now all groups in which the logged in user is present are used when looking for an Image / OriginalFile / FileAnnotation ID. To test, follow scenario from http://trac.openmicroscopy.org.uk/ome/ticket/12146. Try out downloading an Image, FileAnnotation and OriginalFile.

\cc @joshmoore, @sbesson: I'd appreciate feedback with regards to code and test structure - happy to correct if there is too much copypasta.

Blazej Pindelski added 5 commits April 22, 2014 16:17
The addition of omero.group = -1 into the method call context
forces the query to execute across all groups where the
user is a member.
This also handles downloading of images and
file annotations. The changes preserve the
omero.client.uuid string in the context.
@jburel jburel added the dev_5_0 label Apr 28, 2014
@joshmoore
Copy link
Member

$ bin/omero login root@localhost
$ bin/omero group add d1
Added group d1 (id=1503) with permissions rw----
$ bin/omero group add d2
Current group: system
Added group d2 (id=1504) with permissions rw----
$ bin/omero user add d Download Tester d1 d2
Please enter password for your new user (d): 
Please re-enter password for your new user (d): 
Added user d (id=1502) with password
$ bin/omero login d@localhost
Created session 9e849318-a590-4be3-bcbf-0aa0c4f1caad (d@localhost:4064). Idle timeout: 10.0 min. Current group: d1
$ bin/omero upload etc/omero.properties                                                                                                                    
Uploaded etc/omero.properties as 753
$ bin/omero download 753 - | grep omero.version                                                                                    
omero.version=5.0.1-DEV-ice35
$ bin/omero sessions group 1504
Group 'd1' (id=1503) switched to 'd2' (id=1504)
$ bin/omero download 753 - | grep omero.version
omero.version=5.0.1-DEV-ice35

Works great. @bpindelski : my proposed refactoring to getContext() is available in 4dc88e7

@bpindelski
Copy link
Author

@joshmoore Thanks. You beat me to it - I was just about to push almost the same-looking code. Will cherry-pick.

joshmoore and others added 2 commits April 29, 2014 10:28
Added helper method for any omero.client user.
Eventually, kwargs could be present for all of
the uses ("omero.user", "omero.uuid", etc.)
@sbesson
Copy link
Member

sbesson commented Apr 29, 2014

Won't have time to make an extensive review of it but 👍 from my side by looking at it.

@joshmoore
Copy link
Member

@joshmoore Thanks. You beat me to it - I was just about to push almost the same-looking code. Will cherry-pick.

Doh, sorry. Had it built locally for testing and figured might as well.

@bpindelski
Copy link
Author

Would this benefit from waiting on @sbesson's review?

@joshmoore
Copy link
Member

With @sbesson away, let's let him open a PR if he'd like to refactor your tests! 😄 I think with our 4 (6) eyes having been on it, we should be good to go. Merging. If anyone of the PY gateway contributors want to propose expanding the scope of the getContext() method, they need just say so. /cc @will-moore @knabar @aleksandra-tarkowska @cneves

joshmoore added a commit that referenced this pull request Apr 30, 2014
CLI download looks for resources in all user groups (fix #12146).
@joshmoore joshmoore merged commit 1f72845 into ome:dev_5_0 Apr 30, 2014
@bpindelski
Copy link
Author

--rebased-to #2398

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants