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

GitHub Actions: release assets are being uploaded after publishing #5542

Closed
0x2b3bfa0 opened this issue Mar 3, 2021 · 13 comments · Fixed by iterative/setup-dvc#13
Closed
Labels
awaiting response we are waiting for your reply, please respond! :)

Comments

@0x2b3bfa0
Copy link
Member

0x2b3bfa0 commented Mar 3, 2021

Bug Report

Description

Our official setup-dvc GitHub Action fails intermittently in a window of ~10 minutes after each DVC release because they're being published before building and uploading the binary assets.

Reproduce

The following steps are just illustrative and haven't been tested, as this issue is quite hard to reproduce without a flawless coordination or a frequent cron job, like I did; please refer to iterative/setup-dvc#9 (comment) for more information.

  1. Launch a GitHub Actions workflow with the latest setup-dvc GitHub Action:

    on: [workflow_dispatch]
    jobs:
      reproduce:
        steps:
        - uses: iterative/setup-dvc@v1
  2. The GitHub Action code tries to install a 404 page as a Debian package due to some validation shortcomings. 🙃

Expected

A successful run of the GitHub Action.

Environment information

Output of dvc version

Not applicable: this bug keeps users from installing DVC on the GitHub Actions environment.

Additional Information

Would it be possible to keep the releases as drafts until all the assets have been built and uploaded? See the comments on this issue for a general description of the suggested behavior.

Note: it looks like the upload-release-asset GitHub Action we're using for uploading assets is no longer maintained.

@efiop
Copy link
Contributor

efiop commented Mar 3, 2021

@0x2b3bfa0 Good catch! We could indeed start with pre-release, wait for packages to get built and then publish it.

Btw, which binaries are you guys using right now? deb? Maybe you should use https://github.com/iterative/dvc-s3-repo ?

@efiop efiop added the awaiting response we are waiting for your reply, please respond! :) label Mar 3, 2021
@0x2b3bfa0
Copy link
Member Author

0x2b3bfa0 commented Mar 3, 2021

@efiop, we're using both .deb for Ubuntu runners and .pkg for macOS runners. What would be the recommended source for the latter?

@efiop
Copy link
Contributor

efiop commented Mar 3, 2021

@0x2b3bfa0 Ah, I see. Yeah, there is no direct alternative for .pkg. Homebrew might be an option, if you use it already for anything else.

@0x2b3bfa0
Copy link
Member Author

@efiop That sounds like a good plan! I think that we can safely move Ubuntu and macOS runners to use S3 mirror and Homebrew respectively. I'll perform a quick viability check and close this issue if it turns out to be successful.

@0x2b3bfa0
Copy link
Member Author

0x2b3bfa0 commented Mar 3, 2021

It looks like the brew package doesn't have any version but the latest, and our GitHub Action allows users to choose arbitrary versions. Is there any way of installing earlier versions on demand?

@efiop
Copy link
Contributor

efiop commented Mar 4, 2021

@0x2b3bfa0 You could install particular version with brew install dvc@2.0.1. Or am I missing something?

@0x2b3bfa0
Copy link
Member Author

0x2b3bfa0 commented Mar 4, 2021

@efiop Apparently, formulas from homebrew-core usually follow the latest version, determined by the name of the file.

% brew install dvc@2.0.1
Updating Homebrew...
==> Auto-updated Homebrew!
Updated 3 taps (homebrew/core, homebrew/cask and aws/tap).
==> New Formulae
···
==> Updated Formulae
Updated 60 formulae.
==> New Casks
···
==> Updated Casks
Updated 33 casks.
==> Deleted Casks
···

==> Searching for similarly named formulae...
Error: No similarly named formulae found.
Error: No available formula or cask with the name "dvc@2.0.1".
==> Searching for a previously deleted formula (in the last month)...
Error: No previously deleted formula found.
==> Searching taps on GitHub...
Error: No formulae found in taps.

It looks like we're only keeping the latest version on homebrew-core and, as per the official documentation, it wouldn't be possible nor recommendable to keep all the earlier minor versions there:

Homebrew’s versions should not be used to “pin” formulae to your personal requirements. You should instead create your own tap for formulae you or your organisation wish to control the versioning of, or those that do not meet the above standards. Software that has regular API or ABI breaking releases still needs to meet all the above requirements; that a brew upgrade has broken something for you is not an argument for us to add and maintain a formula for you.

Would maintaining a tap be of any use?

@pmrowla
Copy link
Contributor

pmrowla commented Mar 5, 2021

Homebrew only supports latest versions of formulae now. Commands that allowed installing specific versions of packages (like brew switch) have all been deprecated, and even brew pin does not work as expected in a lot of cases. It will keep specific package from being automatically upgraded unless that package is listed as a dependency of something else - in which case homebrew will force upgrading past the pinned version.

As you noted, if we wanted to support specific DVC brew package versions we would have to maintain our own tap, but given that you can already install specific versions of DVC on macos via pip, I would say that pip should just be the preferred installation method here.

@pmrowla
Copy link
Contributor

pmrowla commented Mar 5, 2021

To expand on this, the reason that homebrew only supports latest versions of everything, is so that they don't have to worry about also supporting pinned dependency versions. The dvc brew package currently has 3 dependencies: apache-arrow, openssl@1.1, python@3.9.

(Note that openssl and python are considered "special" cases where homebrew core does support some limited set of @ versions, since other brew packages will depend on a specific version of openssl or python APIs. But even so, over time, homebrew still removes old @ versions of those special packages - currently they only support python@3.7 through python@3.9)

Homebrew will always install the latest version of those dependencies. Eventually, there will be a conflict where the latest version of one or more of those dependencies is incompatible with some range of DVC releases. So to ensure that older DVC releases can always install the required dependencies, we would also have to maintain pinned versions of those dependencies as well.

Essentially, maintaining our own homebrew tap would add an unnecessary burden on core DVC maintainers, given that CML can either wait the 5-10 minutes for the macos .pkg to be uploaded to github or just use pip.

@0x2b3bfa0
Copy link
Member Author

0x2b3bfa0 commented Mar 5, 2021

🙏 Thank you very much for your feedback, @efiop & @pmrowla!

@0x2b3bfa0
Copy link
Member Author

0x2b3bfa0 commented Mar 6, 2021

Essentially, maintaining our own homebrew tap would add an unnecessary burden on core DVC maintainers, given that CML can either wait the 5-10 minutes for the macos .pkg to be uploaded to github or just use pip.

After a painstaking analysis of the provided alternatives, @DavidGOrtega and I have found that using pip would be suboptimal because we would need to override the Python version at the job level, and that could have unintended consequences for the user.

Both waiting for the assets to appear and maintaining a tap would require an extra effort that could probably be avoided just by publishing the releases after the assets get uploaded. What do you think?

@efiop
Copy link
Contributor

efiop commented Mar 7, 2021

@0x2b3bfa0 Btw, not sure how you currently use gha releases, but if you simply check them for the latest one - you could use https://updater.dvc.org, which is our lambda that we maintain and only update when all packages are ready for a release and we are ready to notify viewers. What do you think?

@0x2b3bfa0
Copy link
Member Author

@efiop Awesome! In my opinion, this is exactly what we needed for the sake of simplicity. Sorry for the shortsighted x-y problem; there were quite a few possible alternatives, and none of them was specially compatible with our current workflows.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
awaiting response we are waiting for your reply, please respond! :)
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants