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

chore: allow pulling helmfile releases from new helmfile repository #40

Merged
merged 6 commits into from
Jun 30, 2022
Merged

chore: allow pulling helmfile releases from new helmfile repository #40

merged 6 commits into from
Jun 30, 2022

Conversation

philomory
Copy link
Contributor

Also bumps the default versions of all tools.

Uses the semver-compare library to check whether we are installing version 0.145.0 or later; if so, we use the new repository. Otherwise, we use the old repository.

@philomory
Copy link
Contributor Author

philomory commented Jun 29, 2022

Ah, I realize the problem now: I forgot to include dist/index.js in the commit; which, I mean, it says right there in the readme 🤦

@philomory
Copy link
Contributor Author

Actually, no, there are more problems. I guess in my excitement over the new release I didn't realize they'd also switched to uploading tarballs rather than binaries, and also have changed the filename format.

Having a single action that supports both the old and new repositories may be a bit more cumbersome than I initially thought.

@kondoumh
Copy link
Member

Thank you for the pull request.
I saw the CI log. The HTTP status is 404 at the time of download.

##[debug]Failed to download from "https://github.com/helmfile/helmfile/releases/download/v0.145.0/helmfile_linux_amd64". Code(404) Message(Not Found)
(node:1609) UnhandledPromiseRejectionWarning: Error: Unexpected HTTP response: 404
    at /home/runner/work/setup-helmfile/setup-helmfile/dist/index.js:3831:[25](https://github.com/mamezou-tech/setup-helmfile/runs/7121662968?check_suite_focus=true#step:3:26)
    at Generator.next (<anonymous>)
    at fulfilled (/home/runner/work/setup-helmfile/setup-helmfile/dist/index.js:3741:58)
    at processTicksAndRejections (internal/process/task_queues.js:97:5)

@kondoumh
Copy link
Member

I was able to download it from this URL with curl.

https://github.com/helmfile/helmfile/releases/download/v0.145.0/helmfile_0.145.0_linux_amd64.tar.gz

@kondoumh kondoumh self-requested a review June 29, 2022 23:46
Copy link
Member

@kondoumh kondoumh left a comment

Choose a reason for hiding this comment

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

The lack of arguments for semvercompare was the cause of the CI error.

src/setup.js Outdated Show resolved Hide resolved
@philomory philomory requested a review from kondoumh June 30, 2022 00:04
@kondoumh kondoumh removed their request for review June 30, 2022 01:08
Copy link
Member

@kondoumh kondoumh left a comment

Choose a reason for hiding this comment

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

Hmmm. Can you push dist/index.js as well?

@philomory
Copy link
Contributor Author

philomory commented Jun 30, 2022

Hmmm. Can you push dist/index.js as well?

Done. I don't think I will ever learn to remember to rebuild generated files before pushing 🤦‍♂️

@philomory philomory requested a review from kondoumh June 30, 2022 01:27
Copy link
Member

@kondoumh kondoumh left a comment

Choose a reason for hiding this comment

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

LGTM

@kondoumh kondoumh merged commit bfdc1a0 into mamezou-tech:master Jun 30, 2022
@philomory philomory deleted the feature/update-to-new-helmfile-repo branch June 30, 2022 01:34
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

2 participants