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

Drop omero.version from zip name #6414

Closed
wants to merge 3 commits into from
Closed

Conversation

jburel
Copy link
Member

@jburel jburel commented Nov 4, 2024

This PR removes the version from the zip name to simplify installation when downloading from GH
See ome/omero-install#292 for background

@sbesson
Copy link
Member

sbesson commented Nov 5, 2024

I understand the motivation to have a version-agnostic URL in the context of ome/omero-install#292 but I am concerned about the implications of dropping the version numbers from the server artifact.

The first one that comes to mind is the server upgrade process. With this change, the artifact would become OMERO.server-ice36.zip and be unpacked as OMERO.server-ice36 for any version. Extracting the binaries would then overwrite the same directory by default unless the former directory is itself renamed. In general I anticipate this would need modifications to the whole installation/upgrade instructions and it would be useful to look at the scope of these changes.

@will-moore
Copy link
Member

I wonder if this PR could be causing the build failure on merge-ci today?
https://merge-ci.openmicroscopy.org/jenkins/job/OMERO-server/242/console

00:46:13   inflating: OMERO.server-5.6.3-522-bca4dbbfb5-ice36/sql/psql/OMERO5.4__0/views.sql  
00:46:13 + rm -f OMERO.server.zip
00:46:13 + mv /home/omero/workspace/OMERO-server/OMERO.server /home/omero/workspace/OMERO-server/OMERO.server
00:46:13 mv: cannot stat '/home/omero/workspace/OMERO-server/OMERO.server': No such file or directory
00:46:13 Build step 'Execute shell' marked build as failure

@jburel
Copy link
Member Author

jburel commented Nov 5, 2024

@sbesson
This PR will create zip without version in the name
see https://github.com/jburel/openmicroscopy/releases/tag/5.6.14-test
when you download and the OMERO.server.zip and unzip it, it will create a folder OMERO.server-xxxx-ice3.6 where xxx is the tag name. Only the name of the zip is modified by this PR not the rest i.e. it should not overwrite cf. your comment

@will-moore Probably a small adjustment to the build step but I will exclude the PR for now

@jburel
Copy link
Member Author

jburel commented Nov 5, 2024

--exclude

@sbesson
Copy link
Member

sbesson commented Nov 5, 2024

Understood, and the job log linked from #6414 (comment) confirms that the archive prefix is unmodified and would still include the version of the application. In that case, I would agree the impact of this change will not be as large as I raised. I still suspect some sections of the installation documentations would need to be adjusted

@jburel
Copy link
Member Author

jburel commented Nov 7, 2024

Output of the last 2 commits: https://github.com/jburel/openmicroscopy/releases/tag/5.6.14-test1

@jburel
Copy link
Member Author

jburel commented Nov 7, 2024

Corresponding doc changes ome/omero-documentation#2460

@sbesson
Copy link
Member

sbesson commented Nov 7, 2024

Looks like a67eddf is additionally dropping -ice36 from the prefix folder inside the zip archive. Is that on purpose?

@jburel
Copy link
Member Author

jburel commented Nov 7, 2024

It was on purpose but I am happy to keep it since this will not change between releases and it will be closer to the current redirects we have in place

@sbesson
Copy link
Member

sbesson commented Nov 7, 2024

In addition of the documentation, one workflow that came to mind in the last few days and might be reviewed against these naming changes is the Ansible deployment workflow which is used by IDR and the UoD production OMERO servers amongst other especially https://github.com/ome/ansible-role-omero-server

@chris-allan
Copy link
Member

I appreciate and support the desire to take advantage of GitHub's content delivery network to speed up artifact downloads globally.

That said I'm not a fan of this change as I see it as an end user usability downgrade. Furthermore, at Glencoe we use omero.version extensively and rely on unique artifact names so this change would result in us having to fork build.xml. We, and I expect others, have code that parses the artifact names that would need to be updated to handle what's being proposed here.

Am I correct in my understanding that the change is solely to ensure that omero-install does not need to be updated on a server release?

@jburel
Copy link
Member Author

jburel commented Nov 8, 2024

Thanks for the feedback.
Yes this is mainly to avoid changing the version in omero-install (which is then updating the doc)
To avoid unexpected side effects, I will

  • add a GH action to make sure the version is bumped when a new openmicroscopy tag is pushed
  • run more often the action in the doc repo synch changes from openmicroscopy, omero-install and omeroweb-install.

This should ensure that the version bump occur

@jburel jburel closed this Nov 8, 2024
@chris-allan
Copy link
Member

In case it helps I've also had good success with a redirects.include or similar nginx snippet for consistent naming that gets included in the wider webserver configuration like:

...
location = /omero/5.6/server-latest {
    return 302 https://github.com/ome/openmicroscopy/releases/latest/download/OMERO.server-5.6.13-ice36.zip;
}
...

This has the added advantage of the correct filename being in the result, not having to copy things around, a consistent URL, etc.

@jburel
Copy link
Member Author

jburel commented Nov 8, 2024

@chris-allan I was thinking about that too.
That will be easier. I will adjust the automated script I have put in place to update the redirect to that effect

@jburel
Copy link
Member Author

jburel commented Nov 9, 2024

Humm, I was obviously jet-lag and tired
I already put it in place

rewrite ^/omero/5.6.13/artifacts/OMERO.java-5.6.13-ice36.zip$ https://github.com/ome/openmicroscopy/releases/download/v5.6.13/OMERO.java-5.6.13-ice36.zip last;

I will need to check the sequence of redirects

@jburel
Copy link
Member Author

jburel commented Nov 10, 2024

Actually I had "last" instead of redirect in the config
It should be faster now

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.

4 participants