ome / omero-mapr

An OMERO.web app allowing to browse the data through attributes linked to the image
https://pypi.org/project/omero-mapr/
GNU Affero General Public License v3.0
5 stars 12 forks source link

Django 3 2 #70

Closed will-moore closed 2 years ago

will-moore commented 2 years ago

Updates mapr to work with Django 3.2 and https://github.com/ome/omero-web/pull/356

NB: URLs are case-sensitive. Could cause issues if any keys in mapr_settings.CONFIG are case sensitive?

jburel commented 2 years ago

pytest-django might need to be updated too.

snoopycrimecop commented 2 years ago

Conflicting PR. Removed from build OMERO-plugins-push#20. See the console output for more details. Possible conflicts:

--conflicts

snoopycrimecop commented 2 years ago

Conflicting PR. Removed from build OMERO-plugins-push#1071. See the console output for more details. Possible conflicts:

--conflicts Conflict resolved in build OMERO-plugins-push#24. See the console output for more details.

jburel commented 2 years ago

https://github.com/ome/omero-mapr/blob/master/omero_mapr/apps.py will also need to be changed

jburel commented 2 years ago

cf4168a9eb66ca9a186c1665c7bc643aaea0dc44 is required since other web apps are installed and configured via https://github.com/ome/omero-web-docker/blob/master/standalone/01-default-webapps.omero (docker image using in test-infra). In our case, mapr fails b/c of iviewer and parade. Similar commit should be propagated to the other app repo

sbesson commented 2 years ago

As an alternative to https://github.com/ome/omero-mapr/commit/cf4168a9eb66ca9a186c1665c7bc643aaea0dc44, we could either override OMERO_WEB_IMAGE and/or possible update https://github.com/ome/omero-test-infra/blob/337627dcb1862c85ccc1b1bdca446485f9bea193/.env#L11 so that the base OMERO.web image is used for testing by default. Within the scope of the integration tests of a particular app the base image should be sufficient.

On the other hand, does https://github.com/ome/omero-mapr/pull/70/commits/cf4168a9eb66ca9a186c1665c7bc643aaea0dc44 point to core configuration incompatibilities between our apps?

jburel commented 2 years ago

@sbesson I tried that and it leads to other issues. So I think for now this solution can be applied and we can revisit afterwards if need be.

the main problem is when we are making key changes like Django version. The rest of the time it has no real impact

will-moore commented 2 years ago

PR changes look good and https://merge-ci.openmicroscopy.org/web/mapr/gene/ is working.

pwalczysko commented 2 years ago

can confirm that mapr is fine, testedGene and the top Any KVp", https://merge-ci.openmicroscopy.org/web/mapr/anyvalue/

jburel commented 2 years ago

Proposing to merge and tag as 0.5.0rc1 Any objections?