-
Notifications
You must be signed in to change notification settings - Fork 190
remove assets with the same label #22
base: master
Are you sure you want to change the base?
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM
Nicely done with the added tests.
Any update on merging this? Would be great to have! |
Deleting the old asset and then uploading the new one means there might be some time where the given asset URL gives 404, so it sounds like it's not a completely reliable (in terms of asset availability) way to "update" an asset. It looks like the GitHub API doesn't have a way to automatically update an asset's file though (https://developer.github.com/v3/repos/releases/#edit-a-release-asset can only change the name/label, not the file), so this is probably the closest. |
This is fantastic functionality to have. (Several other CI platforms' "Upload GitHub Release Asset"-type plugins support this functionality.) @IAmHughes: Is this something that can be merged as-is? Or would you rather this be conditioned on an additional (optional, disabled by default?) input flag? EDIT: Based on the API docs:
@gfreezy: Perhaps the order of operations should be:
To resolve one potential race condition (where something else uploads the same name asset between the current check and the call to |
This would be most welcome. This way we can have a single release for nightly builds. |
Any update on this? Would be fantastic for nightly builds. @IAmHughes @gfreezy |
There might be a third check uploading the same asset. I think there should be very few chances existing more than one release processes. So the race condition looks fine to me. I don't know what's needed to be done to get the pr merged. |
As said above, #22 (comment), this technique will leave some time where downloading the asset would fail with 404, because there is some time between deleting the old asset and re-uploading the new asset. So I think it's not fully reliable unfortunately. One solution, if you don't have other downloads in that repository is to make new releases, automatically clean old ones, and use the "latest release" link: https://github.com/OWNER/REPO/releases/latest/download/MY_ASSET.tar.gz |
@eregon, the issue with actually creating new releases (even if you are replacing an existing one) is that notifications are sent to all the watchers of the repo. That is really annoying if the release that is being updated is a nightly pre-release (i.e., run everyday or multiple times a day). In fact, that's what I did in eine/tip and I had to change it because it was unacceptable: eine/tip#111. Nevertheless, I agree that it would be very useful for the API to allow replacing assets, instead of having to have them removed and uploaded again: https://github.com/eine/tip/blob/master/tip.py#L67-L92 Still, I think that removing and re-uploading is reliable enough. E.g., eine/tip is used in ghdl/ghdl to update nightly. Then Action ghdl/setup-ghdl-ci is used to retrieve artifacts/assets from nightly and have the environment configured for users to test their designs. Although possible, we didn't have any 404 issue so far. Sometimes there are 404s or timeouts, but that's normally because of a degraded GitHub service/status, not because the delay in updating the assets. Of course, this might be because assets are really small. If yours are large, you might hit the issue more frequently. |
What about a configurable timeout between deleting the old asset and uploading the new one? Shouldn't be hard to implement and as far as I understand make it work sufficiently well. |
Adding a new asset, then delete the old one and then rename the uploaded one might make it a bit more robust. |
@lazka, unfortunately, the API does not support renaming an asset, does it? |
There is https://developer.github.com/v3/repos/releases/#update-a-release-asset but I've never worked with the github API, so I might be missing something. (I don't want to sidetrack this solution, maybe something for a future improvement) |
With this method download link of latest asset will always work
Possible improvement: |
Yes, I made https://github.com/eregon/publish-release for that. |
When GitHub cares more about renaming their default branch than implementing long-awaited and desperately-needed features... |
Honestly, I suggest everyone use https://github.com/svenstaro/upload-release-action which not only includes this feature: overwriting assets, but also has more awesome features like file glob patterns. |
Is this going to be merged any time soon? It seems to be approved. |
No description provided.