search

QIICR / dcmqi

dcmqi (DICOM for Quantitative Imaging) is a free, open source C++ library for conversion between imaging research formats and the standard DICOM representation for image analysis results
https://qiicr.gitbook.io/dcmqi-guide/
BSD 3-Clause "New" or "Revised" License
228 stars 62 forks source link

Fix tagged release creation and package upload #482

Closed fedorov closed 8 months ago

fedorov commented 8 months ago

Those have been broken since we migrated to GitHub actions :-( Summary below is what I communicated to Vamsi by Discord, but it is better to continue the discussion here for transparency and coordination, so I replicate what I said below.

Some time ago I migrated dcmqi to GA from Appveyor/TravisCI/CircleCI. That was my one and only major foray into GA. I think I got most of the things figured out, with one exception. We used to have the process where tagged git commits that follow the versioning convention would be uploaded to GitHub and a new release would be created, but that got broken, and I feel it would take me too much time to figure out how to fix it.

The original CI was set up by Jean-Christophe @jcfr from Kitware (whom you will hopefully meet at the project week!), and he used a dedicated python package to handle this, see https://github.com/QIICR/dcmqi/blob/master/.github/workflows/cmake-linux.yml#L60-L65. I think the motivation for that package was to deal with the differences across CI platforms.

Packaging and upload of the "latest" release works fine, but not the tagged releases, which I believe would be handled by this clause in the command: --release-packages "build/dcmqi-build/dcmqi-*-linux.zip"

This is the commit that I tagged, which would be expected to generate and upload a release package: https://github.com/QIICR/dcmqi/commit/697ff258b6cc76fcf04d7e1dd35dc67e08f8b857 but it only uploaded latest: https://github.com/QIICR/dcmqi/actions/runs/6671898376/job/18134744092

It may be as simple as the package not assigned the expected file name. As you can see, the package file is named dcmqi-1.2.6-linux-20231027-697ff25.tar.gz, and the release pattern is dcmqi-*-linux.zip. Which as I just now realize can't be right - it should have tar.gz extension. But it also failed on all other platforms to upload the tagged release package.

The reason I would really like to fix it is because yesterday we merged the dcmqi improvement that speeds up DICOM SEG read by orders of magnitude, and I would want to have a release to make this capability easily accessible ("latest" packages are not persistent, so it makes no sense pointing users to include them into notebooks etc).

fedorov commented 8 months ago

Few more bits of information that might be useful. Here are the CI platform specific scripts that were working to create and upload release packages:

It could also be that I did not replicate the steps properly in GitHub actions!

fedorov commented 8 months ago

I should also correct myself - it was not just me who did the migration to GitHub actions - Michael Onken @michaelonken contributed a lot to this as well, helping fixing bugs and addressing many of the related issues as well! 🙏

vkt1414 commented 8 months ago

Happy to work on this! I'll first try to troubleshoot with scikit itself. And if you are okay with using github api, you may consider the following.

In a recent workflow, we tried using GitHub API to handle releases with help of a python script get_latest_index.py running inside GitHub actions update_release_attachment.yml. I also think with the Matrix approach in GitHub actions, we should be able to build and publish for all three platforms parallelly with a single workflow. Might be worth considering as it may reduce maintenance.

fedorov commented 8 months ago

Switching to github API in place of scikit would be completely fine, if fixing the scikit approach is not easy. Using matrix approach is also interesting. The current organization of the workflow is due to the need to support multiple CI platforms before GitHub actions, it was just the most natural first step in migration.

vkt1414 commented 8 months ago

reading scikit documentation carefully suggests that the reason why the commit https://github.com/QIICR/dcmqi/commit/697ff258b6cc76fcf04d7e1dd35dc67e08f8b857 did not trigger release is because the commit was tagged with v1.2.6 where as scikit expects just 1.2.6 I think. which is why the GA logs also says it did not find a release tag image

https://scikit-ci-addons.readthedocs.io/en/latest/addons.html#use-case-automatic-upload-of-release-packages-associated-with-a-tag image

@fedorov I did not test on my fork yet, will test and report back.

fedorov commented 8 months ago

I would be very surprised if that's the culprit. The version tagging has been following the same pattern since the first releases, you can see the tags in https://github.com/QIICR/dcmqi/releases.

image
vkt1414 commented 8 months ago

@fedorov you were right.. tags starting with 'v' were also recognized as released tags.. I found the issue is related to fetching tags in github actions while checking out code. https://github.com/actions/checkout/issues/206 I submitted a PR, as with this PR, it should recognized the tags, but I couldn't check if release attachments are created properly for mac and linux.

vkt1414 commented 8 months ago

update: I made some minor changes to expected release file names and everything should now work as expected.

fedorov commented 8 months ago

Will close once we confirm it works with a release tag - I will make one soon!

fedorov commented 8 months ago

It worked! 🎉

image