German-BioImaging / omero-tagsearch

Extensions to OMERO.web to enhance image/dataset/project filtering from tags.
GNU Affero General Public License v3.0
0 stars 2 forks source link

Renaming the PyPI repo #4

Closed Tom-TBT closed 1 year ago

Tom-TBT commented 1 year ago

I found this: https://github.com/simonw/pypi-rename

My understanding is that it would publish a "new" old-name version of the package (omero-webtagging-tagsearch 3.2.1) with basically nothing inside but a dependency to the newly created package (omero-tagsearch, we could start it at version 3.2.2)

  1. We publish a new release of "omero-tagsearch" version 3.2.2 (no change in the code). We could even do that with a .github/workflows/ from the template of Jean-Marie. with the tag 3.2.2
  2. I install cookiecutter locally pip install cookiecutter and twine pip install twine
  3. I run the cookiecutter on the pypi-rename repo:
    • cookiecutter gh:simonw/pypi-rename
      • new_package_name []: omero-tagsearch
      • old_package_name []: omero-webtagging-tagsearch
      • old_package_new_version []: 3.2.1
    • cd omero-webtagging-tagsearch
    • python setup.py sdist
    • twine upload dist/datasette-insert-api-0.5.tar.gz
Tom-TBT commented 1 year ago

Also, need to rename the new package from omero_webtagging_tagsearch to omero_tagsearch

sbesson commented 1 year ago

Based on the above, my understanding is that you are aiming for a renaming of the project/package/module (removing -webtagging) and a final compatibility release of the omero-webtagging-tagsearch PyPi project.

At first glance, the removal of the webtagging is inline with the split of the parent repository into two. We have been through this type of split in the past and as usual, it includes its lots of pros and cons in terms of usability, upgrade, backwards-incompatibility.

Primarily trying to provide feedback from a consumer perspective, my biggest take-home is that the proposed change should definitely be categorised as backwards-incompatible and reviewed with necessary care given its breaking impact.

For instance, I assume that as part the renaming, the following configuration would need to be updated for registering the app in the OMERO configuration

-omero config append omero.web.apps '"omero_webtagging_tagsearch"'
+omero config append omero.web.apps '"omero_tagsearch"'

From a consumer perspective, the biggest impact of the workflow discussed above is that upgrading a working OMERO.web environment by running pip install -U omero-webtagging-tagsearch will pull omero-tagsearch and most likely leave OMERO.web a broken state as the omero_webtagging_tagsearch no longer exists in the Python environment and cannot be imported. You really need to think carefully about the upgrade workflow to communicate the requirement to update the configuration.

If you want to gain experience with the PyPI publication workflow, I would highly recommend experimenting against https://test.pypi.org/.

Tom-TBT commented 1 year ago

What I could do is add a warning during the installation of omero-webtagging-tagsearch==3.2.1. That won't solve the backward incompatibility issue, but this will add in clarity. Something like:

The package omero-webtagging-tagsearch has been replaced by the package omero-tagsearch since 3.2.2.
This is a breaking change for omero-web, as you need to update the "omero.web.apps" configuration. 
Please refer to #link-to-pypi-package# for detailed upgrade instructions.

But with this method, I think that omero_webtagging_tagsearch will still exist in the python environment. Just that the package is empty. I don't know how that will affect OMERO.web (fail at start? error when using the plugin?).

I'll communicate the breaking change on image.sc, on this Github repo and on both pypi (omero-webtagging-tagsearch points to omero-tagsearch).

In the new doc, I found these two pages:

I haven't found a reference in the omero guides.

There are also references in the legacy doc, but we don't update that right?

sbesson commented 1 year ago

Looks like the cleanest upgrade workflow might indeed look along the lines of

# stop omero web
pip uninstall omero-webtagging-tagsearch
pip install omero-tagsearch
omero config set omero.web.apps ...

# start omero web

Introducing either an Upgrading document or a dedicated section in the Readme repository that contain detailed instructions and can be referred to sounds valuable.

Another practical question: are you following the existing versions (in which case I'd suggest to call this 4.0.0) or are you starting back e.g. at omero-tagsearch 1.0.0?

Glencoe doc, has a dead link to install instruction

This looks like a mirror of the legacy OME help for OMERO 5.2. This version is EOL so this documentation set can be safely ignored

Tom-TBT commented 1 year ago

I added stop/start omero web to the upgrade doc I made in the ongoing PR. But for the omero config, I'd rather advise to use omero edit to manually update the name, instead of setting the updated config including all the other plugin config.

I was going with version 3.2.2, but I will follow you advice and go for 4.0.0

Tom-TBT commented 1 year ago

So it seems that it's not possible to simply display a warning message when installing from pip. However, I can make the install fail with an error pointing to the new package:

(deleteme) >pip install -i https://test.pypi.org/simple/ omero-webtagging-tagsearchtom==3.2.6
Looking in indexes: https://test.pypi.org/simple/
Collecting omero-webtagging-tagsearchtom==3.2.6
  Using cached https://test-files.pythonhosted.org/packages/e2/d2/18459e6e11cefb0c4cd617879af87e5924353db7e90bed6c19b703d99efe/omero-webtagging-tagsearchtom-3.2.6.tar.gz (1.4 kB)
  Preparing metadata (setup.py) ... error
  error: subprocess-exited-with-error

  × python setup.py egg_info did not run successfully.
  │ exit code: 1
  ╰─> [5 lines of output]

      The package omero-webtagging-tagsearch has been replaced by the package omero-tagsearch since 4.0.0.
      This is a breaking change for omero-web, as you need to update the "omero.web.apps" configuration.
      Please refer to #link-to-pypi-package# for detailed upgrade instructions.

      [end of output]

  note: This error originates from a subprocess, and is likely not a problem with pip.
error: metadata-generation-failed

× Encountered error while generating package metadata.
╰─> See above for output.

note: This is an issue with the package mentioned above, not pip.
hint: See above for details.

To achieve that, I added the following code snippet in setup.py:

message = """ the message """
if not 'sdist' in sys.argv:
    sys.exit(message)

and only provided the source distribution:

>python setup.py sdist
>twine upload -r testpypi dist/*

What do you think about this approach? I think it's better than failing silently

sbesson commented 1 year ago

Trying to summarize my understanding of the situation:

The reference PEP for handling similar scenarios seems to be https://peps.python.org/pep-0423/#how-to-rename-a-project. This is essentially the approach of the the pypi-rename template i.e. publish a final release with the correct metadata which would uninstall the old package and transitively install the new supported packages (or both new ones in that case). It looks like you are mostly there. What are the remaining issues you are trying to solve?

Tom-TBT commented 1 year ago

You summarized it well.

I added the Obsoletes-dist to my PRs' TODO list. My current solution is pretty much following the PEP (the old package has a different name, everything dropped in the next "empty" release). But the PEP is not covering the topic of breaking change between old-name and new-name (for the change of config in OMERO.web). But I think the "error at install" solution is the most obvious way to notify that the upgrade has to be taken care of manually (or it keeps the old working version).

For tagsearch PR, I think I'm pretty much ready.

For autotag PR, I still have to test it. I had an unsuccessful time yesterday building the JS with NPM locally, but I'll try again.

But at least I tested both Github workflows, and the builds are successful (if I ignore the security issues shown by NPM for this "change of name" release).

Tom-TBT commented 1 year ago

I tested both PR, my todo list is ~complete:

@sbesson @joshmoore Do you want to review/test the PRs? Or anyone else we should ask? (eg @jburel, @will-moore )

jburel commented 1 year ago

@Tom-TBT I will review when I am back to office

sbesson commented 1 year ago

@Tom-TBT catching up after a week of annual leave, it would be useful to have some feedback from Jean-Marie and the Dundee team. I can also try and schedule some minimal testing of the upgrade process on some of our dev instances in the upcoming 2 weeks.

I think it's going to be hard to properly test the whole workflow without making the packages available via PyPI. To avoid a form of a chicken and egg problem, would you consider updating the versions of https://github.com/German-BioImaging/omero-tagsearch/pull/5 and https://github.com/German-BioImaging/omero-autotag/pull/6 to release them as 4.0.0rc1 release candidates on PyPI instead of full releases?

jburel commented 1 year ago

Another option if you do not want to use an rc is to use testpypi for example https://github.com/TheJacksonLaboratory/ezomero/blob/main/.github/workflows/upload_to_test_pypi.yml This is not a strategy we use. You will need a separate account for testpypi

Tom-TBT commented 1 year ago

So, I have both tagsearch and autotag on the test.pypi I uploaded them with twine manually:

Is that enough for you to test? Or do I need the release workflow too? I tested the building in separated PRs, so the workflows are working (at least up to the upload to pypi).

jburel commented 1 year ago

@Tom-TBT Thanks I will update the merge-ci job and install from test.pypi

jburel commented 1 year ago

I added to merge-ci

Downloading https://test-files.pythonhosted.org/packages/5f/16/99a3a7cc19ec7662270ef6b61e65d4be8889b98b9803d4920b69529a6317/omero-autotag-4.0.3.tar.gz (595 kB)
15:16:47      ��������������������������������������������������������������������������������������������������������������������� 595.5/595.5 kB 6.0 MB/s eta 0:00:00
15:16:47   Installing build dependencies: started
15:16:48   Installing build dependencies: finished with status 'error'
15:16:48   error: subprocess-exited-with-error
15:16:48   
15:16:48   �� pip subprocess to install build dependencies did not run successfully.
15:16:48   ��� exit code: 1
15:16:48   ������> [3 lines of output]
15:16:48       Looking in indexes: https://test.pypi.org/simple/
15:16:48       ERROR: Could not find a version that satisfies the requirement setuptools>=40.8.0 (from versions: none)
15:16:48       ERROR: No matching distribution found for setuptools>=40.8.0
15:16:48       [end of output]
15:16:48   
15:16:48   note: This error originates from a subprocess, and is likely not a problem with pip.
15:16:48 error: subprocess-exited-with-error
jburel commented 1 year ago

Context: merge-ci is running on CentOS 7 with Python 3.8 (SCL) We have also upgraded Django to 4.2.2

pip install -i https://test.pypi.org/simple/ omero-tagsearch works but changes are required in order to work with Django 4.2.2. See for example https://github.com/ome/omero-figure/pull/508/files for changes to make

pip install -i https://test.pypi.org/simple/ omero-autotag does not work. It fails with the error mentioned above This could be related to the fact (not tested) that the name in package.json is "name": "webtagging-autotag",. This should probably be "name": "autotag",

jburel commented 1 year ago

https://github.com/jburel/omero-autotag/tree/workflow_pypi allows to pass the error when installing from testpypi I have not made the changes yet for Django

sbesson commented 1 year ago

@Tom-TBT is the intent to combine the repository renaming/transfer and Django 4.2 support into the upcoming release? If so, I will wait on our side until we have a release candidate version of OMERO.web with Django 4.2 support before engaging some testing on the upgrade workflow. Otherwise, happy to give the existing packages a try using the released stack (Django 3.2, Python 3.8+).

jburel commented 1 year ago

I can also test with Django 3.2 and Python 3.6

jburel commented 1 year ago

https://github.com/jburel/omero-tagsearch/tree/workflow_pypi and https://github.com/jburel/omero-autotag/tree/workflow_pypi are now deployed on merge-ci (Python 3.8, Django 4.2) Django 4 adjustments are limited.

jburel commented 1 year ago

@Tom-TBT if you want to test on merge-ci, let us know i will send you info to access it

pwalczysko commented 1 year ago

On merge-ci, the Tagsearch does not work.

  1. Select a tag ("Screenshot")
  2. Observe that there is an endless spinner in central pane, and an empty error window pops up in webclient
  3. From console, extract following error

CSRF Error. You need to include valid CSRF tokens for any POST, PUT, PATCH or DELETE operations. You have to include CSRF token in the POST data or add the token to the HTTP header."

Tom-TBT commented 1 year ago

Thanks all for the testing. @jburel I built and uploaded your branches to testpypi: https://test.pypi.org/project/omero-tagsearch/4.0.2/ https://test.pypi.org/project/omero-autotag/4.0.4/

@sbesson changes to Django 4.2 have already been done by @jburel so if the test works, I guess those could also be included in the first release.

jburel commented 1 year ago

Clarification: Changes for 4.0 have been made, support. 4.2 might require a second round of adjustment (I doubt)

sbesson commented 1 year ago

Happy to go with this approach. A release of OMERO.web 5.22.0 is planned early next week with Django 4.2 support.

I can schedule the testing of the tagsearch/autotag RC upgrade in our existing deployment both with and without the OMERO.web/Django upgrade and report next week

jburel commented 1 year ago

@Tom-TBT Tested on merge-ci with Python 3.8 and Django <4.1 https://test.pypi.org/project/omero-tagsearch/4.0.2/ https://test.pypi.org/project/omero-autotag/4.0.4/ I can now install the packages without problem Tag Search works as expected i.e. no CSRF Error

jburel commented 1 year ago

Tested on merge-ci with Python 3.8 and Django == 4.2.2 https://test.pypi.org/project/omero-tagsearch/4.0.2/ https://test.pypi.org/project/omero-autotag/4.0.4/ I can now install the packages without problem

Tag Search does not work. This should be fixed in a second release

sbesson commented 1 year ago

Performed some minimal testing on one of our dev instances using the two following versions from testpypi

omero-autotag==4.0.4
omero-tagsearch==4.0.2

together with Django 4.2.3 and the freshly released OMERO.web 5.22.0. The OMERO configuration was updated as per the instructions. After successfully reloading the OMERO.web service, both applications worked as expected. Tags could be added/removed using the autotag panel and searched using the tagsearch endpoint.