ome / omero-py

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

config add web middleware #280

Closed will-moore closed 3 years ago

will-moore commented 3 years ago

See https://github.com/ome/omero-web/pull/260

With an update to the default omero.web.middleware as in the PR above, if the user has previously updated their config then the values will be stored in the etc/grid/config.xml and the new default will be ignored. This PR aims to update the omero.web.middleware config in the same way as the above PR. This will only happen if the above PR is included in omero-web (the new middleware is importable)

NB: last commit of the PR above changes the default omero.web.middleware setting so that it is simpler for this PR to update in the same way (simply append a single class, instead of update multiple classes in the list). This allows us to simplify the update and reduce the amount of settings that are hard-coded in config.py.

I'm assuming no-one is upgrading from "4.2.1" (6 years ago).

To test:

NB: If you ONLY stop web, upgrade web and start web (without any $ omero config... command), the change will not take effect.

will-moore commented 3 years ago

@joshmoore I could maybe do a better fix by creating a INSTALLED_MIDDLEWARE and ADDITIONAL_MIDDLEWARE in https://github.com/ome/omero-web/pull/260 (just as we have for omero.web.apps with INSTALLED_APPS and ADDITIONAL_APPS). Then use this config.py update to only leave user-configured middleware in the omero.web.middleware (ADDITIONAL_MIDDLEWARE) list. I might give it a try and see how it looks... Don't know if/when we might want to apply this to other configs. There's not many that we are likely to update.

joshmoore commented 3 years ago

Don't know if/when we might want to apply this to other configs.

Are there others where we are copying a default?

will-moore commented 3 years ago

I could list them. Looking for where we use json.loads and the default isn't empty:

"omero.web.caches": '{"default": {"BACKEND":' ' "django.core.cache.backends.dummy.DummyCache"}}'
"omero.web.server_list": '[["%s", 4064, "omero"]]' % CUSTOM_HOST
"omero.web.open_with": '[["Image viewer", "webgateway", {"supported_objects": ["image"],"script_url": "webclient/javascript/ome.openwith_viewer.js"}]]'
"omero.web.ui.top_links": (Data, History, Help)
"omero.web.ui.metadata_panes": (tag, map, table, file, comment, rating, other)
"omero.web.ui.right_plugins":  (Aquisition, Preview)

I don't expect many of those defaults to change, or for users to have them configured. Except maybe top_links and open_with. BUT, if we only allow users to ADD to the defaults, then they can't remove items from defaults. E.g. remove metadata_panes. The same applies to omero.web.middleware. If someone has already removed an item from the default settings, then putting all the defaults into a non-editable list would add it back. So we can't support remove of defaults AND auto update of defaults. I think I'll try the separation of omero.web.middleware into 'installed' and 'additional' lists, and we can see what that looks like. Then we can decide if we want to do the same with open_with and/or top_links.

will-moore commented 3 years ago

With https://github.com/ome/omero-web/pull/260/commits/70d22293e45a76d2fd47ab56f48bc41f61688719 and the last commits above, the user settings are now distinct from the default middleware.

joshmoore commented 3 years ago

@sbesson was kind enough to scour the history to find https://github.com/ome/openmicroscopy/pull/2030#issuecomment-33578012 -- pasting it here for reference.

will-moore commented 3 years ago

I don't think we're going to be able to apply the same approach to the other settings lists above. For the UI settings (and open_with), it's possible a user will have configured them to remove one or more default items. For web.caches, this isn't a list, so if it's been configured it will likely be set rather than append to default. I guess for server_list, we might want to change the one default option at some point. But I think we could leave any upgrade path in config.py till it's needed.

will-moore commented 3 years ago

With https://github.com/ome/omero-web/pull/260 closed for now, don't need this currently. Closing...