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

upload assets on tag #6318

Merged
merged 12 commits into from
Jul 26, 2022
Merged

upload assets on tag #6318

merged 12 commits into from
Jul 26, 2022

Conversation

jburel
Copy link
Member

@jburel jburel commented Apr 28, 2022

Follow up from 5.6.4 release.
This PR adds steps to the build workflow to avoid the manual migration of the artifacts from downloads.o.org to GitHub.

The workflow does the following, When a tag is pushed:

  • The artifacts are built
  • A release is created
  • The artifacts are uploaded

I have opted for one single SHASUMS and MD5 collecting the various values to simplify the selection and reduce the number of uploaded files.

This can replaced the "release" job on CI

See https://github.com/jburel/openmicroscopy/releases/tag/v5.6.4-m1

This was referenced Apr 28, 2022
@sbesson
Copy link
Member

sbesson commented Apr 29, 2022

Immediate thoughts:

  • using the GitHub release API as the primary source of artifacts feels like a valuable move following the same line of argument as Rtd omero-documentation#2195 (comment),
  • comparing to https://downloads.openmicroscopy.org/omero/5.6.4/artifacts/, the staging release linked above above omits openmicroscopy-5.6.4.zip. Or is it equivalent the source code zip?
  • both the build number and the ice separator are removed from the bundle names. I think the former makes sense as it was related to the Jenkins release infrastructure. The latter was meaningful when several Ice versions were concurrently supported so as long as we accept this will not be the case moving forward, it makes sense as a simplification.
  • With the name simplification, the proposal defines a fairly simple URL for server artifacts https://github.com/ome/openmicroscopy/releases/download/v<version>/OMERO.server-<version>.zip
    • is the expectation that all downstream consumers (omero-install, omero-documentation, omego, www.openmicroscopy.org) will be updated to start consuming this new URL? or would we retain some mirroring logic and leave some dependencies on downloads.openmicroscopy.org/omero/?
    • this also raises the timeliness of reviewing the repository name as mentioned in Rtd omero-documentation#2195 (comment)

@jburel
Copy link
Member Author

jburel commented Apr 29, 2022

  • openmicroscopy-vXX.zip is equivalent to the source.zip. Note that it is not available on GitHub on the tag 5.6.3/5.6.4
  • downstream repositories will consume the zip from GitHub (as it was for omero-install prior to 5.6.4). I don't think there is too much to gain to mirror to downloads.o.org
  • When we have doc on RTD, changing the link and updating the doc will be straightforward but there are not really coupled. Changing to the various repositories could be done ahead of time. This could potentially be done via an action so we keep repositories in synch i.e. new openmicroscopy tag, PR opened in another repo similar to what we do for the doc and omeroweb-install and omero-install

@joshmoore
Copy link
Member

One thing to keep in mind that I went through with https://github.com/zarr-developers/zarr-logo/pull/2/commits is that on download things get automatically zipped.

Also: I can highly recommend experimenting with the gh release commands to make sure what's being upload and downloaded matches your expectations.

@sbesson
Copy link
Member

sbesson commented Apr 29, 2022

Another component which I forgot to mention in my previous comment is our Ansible deployment stack especially ome.omero_server and the underlying omego library currently rely on the current https://downloads.openmicroscopy.org/omero layout and naming for retrieving the server artifacts. Amongst other, this proposal might force us to revisit ome/omego#127 to download components directly from GitHub.

Something to think about is the breaking impact on deployment if a new library/role release or new scripts are required to upgrade a server. I don't consider it to be a blocker per se but it might lead us to a dedicated upgrade documentation page similar to what we did for the Python3 migration.

@sbesson sbesson mentioned this pull request Jun 22, 2022
This was referenced Jun 23, 2022
@jburel
Copy link
Member Author

jburel commented Jun 28, 2022

Reactivate that work

@jburel
Copy link
Member Author

jburel commented Jun 28, 2022

Add openmicroscopy-XXX.zipsee https://github.com/jburel/openmicroscopy/releases/tag/v5.6.5-m2

@jburel
Copy link
Member Author

jburel commented Jun 29, 2022

cf. #6318 (comment)
Re-activated see ome/omego#132.

Copy link
Member

@sbesson sbesson left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Given the OMERO.server 5.6.5 experience, I am definitely in favor of inverting the release workflow i.e. migrate the artifact generation to GitHub Actions and mirror these artifacts under downloads.openmicroscopy.org.

A few inline comments and a couple of questions:

  • the new process seems to fully drop the build suffix. Is that on purpose and do we know whether this might cause impact downstream?
  • we might need to review whether this GH based process might affect a security workflow

.github/workflows/release.yml Outdated Show resolved Hide resolved
.github/workflows/release.yml Outdated Show resolved Hide resolved
.github/workflows/release.yml Show resolved Hide resolved
@jburel
Copy link
Member Author

jburel commented Jun 29, 2022

A few inline comments and a couple of questions:

  • the new process seems to fully drop the build suffix. Is that on purpose and do we know whether this might cause impact downstream?

I can't get a build number as bXXXX is internal to our CI. The build is now happening in GHA.

@sbesson
Copy link
Member

sbesson commented Jun 29, 2022

I can't get a build number as bXXXX is internal to our CI. The build is now happening in GHA.

I assume in the GitHub context github.run_id would be the closest equivalent. As mentioned above, I do not know of the impact of dropping the build. it An advantage is that would certainly make the download URL fully predictable from the OMERO.server version.

@jburel
Copy link
Member Author

jburel commented Jun 30, 2022

Using the job id is an option

@jburel
Copy link
Member Author

jburel commented Jun 30, 2022

https://github.com/jburel/openmicroscopy/releases/tag/v5.6.5-m3 using the run_id in the name
This should match the convention currently used when the artifacts are built via Jenkins CI

@jburel
Copy link
Member Author

jburel commented Jul 1, 2022

Looking closely, I don't think the build number needs to be kept.
I am also looking into improving ome/omego#132 so we can pass major or major.minor so we can install the latest version from a major version without having to update the link

the usage of get_latest_runs will need to be reviewed. This is mainly (only!) used to retrieve a latest run from Jenkins latest.

@jburel
Copy link
Member Author

jburel commented Jul 1, 2022

Looking at another part, candidate for automation, not including the build number will allow to create the url by finding the latest release tag from the GitHub API . Including the build number will not permit that.

@jburel jburel mentioned this pull request Jul 4, 2022
run: |
set -x
assets=()
touch SHASUMS
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

How does the MD5 file that's in your tag get created? Do we need both?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@sbesson and I discussed it and decided to remove the MD5

@jburel
Copy link
Member Author

jburel commented Jul 5, 2022

Copy link
Member

@sbesson sbesson left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

One outstanding question about simplifying the for loop.

On the build number suffix, my understanding is that the impact is rather about cleaning up various locations and I am in favor of dropping it.
It partly raises the question of how a rebuild should be handled using GitHub Actions. I assume pushing a new tag?

.github/workflows/release.yml Outdated Show resolved Hide resolved
@jburel
Copy link
Member Author

jburel commented Jul 6, 2022

For the build number, it will be matter of adjusting a few places. If we agree, I will roll back and do not add the build number
A rebuild will be happening by pushing a new tag.
One downside of this approach is if we want to retract a release. For that eventually e.g. major releases, rc will be used or possible intermediate dev tags.

@jburel
Copy link
Member Author

jburel commented Jul 6, 2022

Last 2 commits

  • Remove the build number
  • simplify the code. All actions occur in the target directory so the name in SHASUM does not content target/NAME

Tested via https://github.com/jburel/openmicroscopy/releases/tag/5.6.5-m5

@jburel
Copy link
Member Author

jburel commented Jul 11, 2022

Any more comments on this PR?

@sbesson
Copy link
Member

sbesson commented Jul 12, 2022

Is the missing leading 5 in https://github.com/jburel/openmicroscopy/releases/tag/5.6.5-m5 on purpose?

@jburel
Copy link
Member Author

jburel commented Jul 12, 2022

oh. Let me check. That should not be the case.

Edit: This is because I did not tag with vX.X.X and I remove the first character i.e. tag_value="${tag_name:1}-ice36"
I will add check if it starts with v to avoid such mistake

@jburel
Copy link
Member Author

jburel commented Jul 12, 2022

The last commit has been tested using 2 tags. One starting with v and one without v
https://github.com/jburel/openmicroscopy/releases/tag/5.6.5-m7
https://github.com/jburel/openmicroscopy/releases/tag/v5.6.5-m6

Copy link
Member

@sbesson sbesson left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ready to test in the upcoming server release from my perspective

@jburel jburel merged commit 8d20583 into ome:develop Jul 26, 2022
@jburel jburel deleted the upload_artifacts branch March 10, 2023 19:58
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.

None yet

3 participants