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

OMERO.web client settings override server client settings #6053

Merged
merged 2 commits into from
Jul 3, 2019

Conversation

manics
Copy link
Member

@manics manics commented Jun 12, 2019

What this PR does

If omero.client.* properties are set in OMERO.web they override the corresponding omero.client.* settings sent by the server.

Testing this PR

  1. Setup OMERO.web, connect to a different server with the default omero.client.ui.tree.orphans.name setting
  2. Login to OMERO.web, you should see the Orphans folder is called Orphaned images
  3. Stop OMERO.web. You may or may not need to delete the OMERO /var directory (not sure why)
  4. omero config set omero.client.ui.tree.orphans.name whatever
  5. Start OMERO.web
  6. The Orphans folder should be called whatever

Note if you want to test the setting in the trello card omero.client.ui.tree.orphans.enabled=false you need to read the code to figure out in what situations it will hide the folder: https://github.com/openmicroscopy/openmicroscopy/blob/v5.5.0-rc3/components/tools/OmeroWeb/omeroweb/webclient/views.py#L667-L670

I was going to add a test but I couldn't figure out how.... if anyone has a suggestion let me know (or we could skip adding a test)

@will-moore
Copy link
Member

Works fine and code looks good.
Don't know a way to test this.

When I set this locally:

 omero config set omero.client.ui.tree.orphans.enabled False

then connected to eel and viewed e.g. "All Members" of a group that I wasn't an owner of, the Orphans folder didn't show up in the tree.

Good to merge.

@joshmoore
Copy link
Member

NB: this is the first time that web code is starting to depend on omero.plugins code. As things currently stand, that's fine & permissible, but with the upcoming refactoring, we might want to consider if this is safe. (Also, when we decouple, will it lead to any issues)

A simple fix for the import issue would be to refactor open_config elsewhere, but that carries with it various issues i.e. who knows about /etc/grid/templates. It might be time to consider whether or not we would also decouple OMERO.web from /etc/grid when we decouple the repositories.

@manics
Copy link
Member Author

manics commented Jun 18, 2019

How about get rid of the config.xml in everything apart from the server where it's required, and have a new config.json (or config.d/*.json?) for OMERO.web? Several of the more complicated web config parameters are already JSON but they have to be reformatted/quoted as a single-line string for omero config, so this would make things clearer.

@joshmoore
Copy link
Member

@manics: agreed. Something like that (perhaps keeping in mind the type of configuration that we will want for the vertx microservices) would be great. Obviously a question of how to get existing installations from where they are to that new system.

@manics
Copy link
Member Author

manics commented Jun 19, 2019

Do it as part of the Python 3 decoupling/pypi work and make a complete break? If OMERO.web is pip-installable a relative /etc/grid/config config directory doesn't make sense so that would need refactoring anyway- might as well use it as an opportunity to replace it.

@joshmoore
Copy link
Member

Definitely agreed that that's the right time to do it, but under "a complete break" I wouldn't say we can ignore the short-term impact on users (how they upgrade) nor the long-term (as we move to microservices).

@joshmoore
Copy link
Member

Merging.

I was going to add a test but I couldn't figure out how.... if anyone has a suggestion let me know (or we could skip adding a test)

@manics : can I suggest an example repo where the overriding makes sense? Perhaps a public & non-public front-end set up like omero.lifesci.

@joshmoore joshmoore merged commit bf319d3 into ome:develop Jul 3, 2019
@joshmoore joshmoore added this to the 5.5.1 milestone Jul 3, 2019
@manics manics deleted the omero•web•client•settings branch July 5, 2019 16:42
@manics
Copy link
Member Author

manics commented Jul 17, 2019

See #6086 for followup of the config discussion

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.

3 participants