Skip to content

Api experimenters groups#5315

Closed
will-moore wants to merge 38 commits intoome:developfrom
will-moore:api_experimenters_groups
Closed

Api experimenters groups#5315
will-moore wants to merge 38 commits intoome:developfrom
will-moore:api_experimenters_groups

Conversation

@will-moore
Copy link
Copy Markdown
Member

@will-moore will-moore commented May 30, 2017

What this PR does

Adds initial support for /experimenters/ and /experimentergroups/ to the JSON API.

This includes updating the getQueryString() for the BlitzGateway ExperimenterWrapper and ExperimenterGroupWrapper classes, so that by default they load experimenters without groups and load groups without experimenters.

Need to use the opts to load experimenters / groups (same as for Image 'load_pixels' etc).

conn.getObjects("ExperimenterGroup", opts={load_experimenters=True})
conn.getObjects("Experimenter", opts={load_groups=True})

In several places in webadmin we are using conn.getObjects("ExperimenterGroup") to list groups (e.g. in the experimenter/new/ form) when we don't need to load_experimenters. Now that these are not being loaded, the loading time for that page is ~2 x faster for me.

I've also updated the group.copyGroupExperimenterMap() and experimenter.copyGroupExperimenterMap() to lazy load if not previously loaded.
This means that doing the following is slower because of lazy loading, but at least it won't fail

for e in conn.getObjects("Experimenter"):
    e.copyGroupExperimenterMap()
    # or this also needs to load groups
    e.isAdmin()

TODOs:

  • Support for single Experimenter or Group
  • Tests
  • Loading experimenterGroupMap etc?? (see below)
  • Query experimenters or groups by name? E.g. to look up your own experimenter ID from omeName or lastName? E.g. /experimenters/?omeName=will

Testing this PR

  1. Browse to /api/v0/m/experimenters/ to list all.
  2. filter with ?group=3 or with /api/v0/m/groups/3/experimenters
  3. Browse to /api/v0/m/experimentergroups/ to list all.
  4. filter groups with ?experimenter=3 or with /api/v0/m/experimenters/3/experimentergroups
  5. Follow links to see single experimenter or group: /api/v0/m/experimenters/3/ or /api/v0/m/groups/3/
  6. Need to test webclient and webadmin in places where we are loading experimenters / groups to check that we don't get unloaded exceptions:
    • Main /webclient/ page - Groups/Users menu
    • Search page (groups select option)
    • webadmin listing and editing groups
    • webadmin listing and editing experimenters
    • User settings in webclient (including for group leaders)

Related reading

To discuss:

  • Do we expose the full name experimenterGroups anywhere (urls, filters etc) or do we simply stick with groups?
  • Currently omero-marshal doesn't support loading groupExperimenterMap for Experimenters OR Groups. This means you can't choose to list Groups with Experimenters loaded (or list Experimenters with Groups loaded). Also, can't see which members are Owners of a Group.

cc @chris-allan @jburel

@will-moore
Copy link
Copy Markdown
Member Author

Closing until after #POTN and more complete...

@will-moore will-moore closed this May 30, 2017
@will-moore will-moore reopened this Jun 21, 2017
@jburel
Copy link
Copy Markdown
Member

jburel commented Jun 21, 2017


./components/tools/OmeroWeb/omeroweb/api/urls.py:327:24: E127 continuation line over-indented for visual indent
./components/tools/OmeroWeb/omeroweb/webadmin/views.py:102:80: E501 line too long (82 > 79 characters)

@will-moore will-moore force-pushed the api_experimenters_groups branch from 6aef59b to 24ad4e5 Compare June 21, 2017 22:49
@jburel jburel added the develop label Jun 23, 2017
@mtbc
Copy link
Copy Markdown
Member

mtbc commented Jul 3, 2017

excluding to let #5349 through now #5101 is in

@mtbc mtbc added the exclude label Jul 3, 2017
@will-moore
Copy link
Copy Markdown
Member Author

@jburel Could you remove the "exclude" label from this PR? Thanks

@jburel jburel removed the exclude label Aug 15, 2017
will-moore added a commit to will-moore/openmicroscopy that referenced this pull request Aug 18, 2017
…ap()

In PR ome#5315 this now lazy loads group experimenters if unloaded
if d.child.id.val != self.getUserId():
yield ExperimenterWrapper(self, d.child)

def groupSummary(self, gid=None, exclude_self=False):
Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Seems that webtagging is still using this.
So I will leave it until webtagging has updated.

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Agreed, but removal of any method from omero.gateway should include a deprecation so if you foresee dropping it, perhaps add the deprecation now?

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It has been marked as deprecated for some time already. That's why I decided to remove it.

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Apologies, you're of course right. I had confused this with https://github.com/openmicroscopy/openmicroscopy/pull/5402/files#diff-5586c69038567a9c1960ae4f6d731278L509 where I convinced myself that the removal was less critical since it was in webclient

@rgozim
Copy link
Copy Markdown
Member

rgozim commented Aug 30, 2017

API's tested and working well.

Only concern, as noted by @will-moore, might be the ability of a public user being able to view experimenter and experimenter group data via the API (no authentication required to query the API).


Returns a tuple of (query, clauses, params).
Supported opts: 'group': <group_id> to filter by ExperimenterGroup
'load_groups': <bool> to load ExperimenterGroups'
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

extra '

group.
Returns a tuple of (query, clauses, params).
Supported opts: 'experimenter': <experimenter_id> to filter by
ExperimenterGroup
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

filter by Experimenter

Returns a tuple of (query, clauses, params).
Supported opts: 'experimenter': <experimenter_id> to filter by
ExperimenterGroup
'load_experimenters': <bool> to load Experimenters'
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

extra '

@jburel
Copy link
Copy Markdown
Member

jburel commented Aug 30, 2017

That is a serious concern

@will-moore
Copy link
Copy Markdown
Member Author

Hmmm - let's discuss the public user issue. If you've configured the public url regex then chances are that /api/ urls are not public unless desired. E.g. they're not public on IDR.

@joshmoore
Copy link
Copy Markdown
Member

It would be useful if a API definition itself (whether this one, or one introduced by third parties) could have some "awareness" of its critical nature so that the burden isn't externalized to the sysadmin to always have a regex set correctly.

@will-moore
Copy link
Copy Markdown
Member Author

Re: discussion of guest user.
Logging in as guest, then using the session ID to access the JSON API gives the same exception as trying to load data via the CLI:

$ bin/omero user list
Using session 2960bf65-c0b4-4748-8b47-40eac3fdc22f (guest@localhost:4064). Idle timeout: 10 min. Current group: guest 
...
  File "/Users/wmoore/Desktop/OMERO/openmicroscopy/dist/lib/python/omero_api_IAdmin_ice.py", line 864, in lookupExperimenters
    return _M_omero.api.IAdmin._op_lookupExperimenters.invoke(self, ((), _ctx))
omero.SecurityViolation: exception ::omero::SecurityViolation
message = No matching roles found in [guest] for session 2960bf65-c0b4-4748-8b47-40eac3fdc22f (allowed: [user])

also seen at:
/api/v0/m/projects/?bsession=2960bf65-c0b4-4748-8b47-40eac3fdc22f&server=1

@will-moore
Copy link
Copy Markdown
Member Author

Need more discussion of experimenters/groups visibility for public & other users.
No need to get this into 5.4.0, closing for now...

@will-moore will-moore closed this Sep 4, 2017
This was referenced Sep 4, 2017
@will-moore will-moore deleted the api_experimenters_groups branch February 18, 2019 04:13
@will-moore will-moore mentioned this pull request Apr 4, 2019
will-moore added a commit to will-moore/openmicroscopy that referenced this pull request Jul 4, 2019
will-moore added a commit to will-moore/omero-web that referenced this pull request Mar 6, 2020
will-moore added a commit to will-moore/omero-web that referenced this pull request Mar 6, 2020
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.

5 participants