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

fix(helm): Fix baking of helm artifacts #442

Merged
merged 1 commit into from
Oct 10, 2019
Merged

fix(helm): Fix baking of helm artifacts #442

merged 1 commit into from
Oct 10, 2019

Conversation

ezimanyi
Copy link
Contributor

Fixes spinnaker/spinnaker#4874.

We currently throw an exception if a helm artifact doesn't have a reference. We're only using that reference to generate a file name to use when downloading the artifact; helm artifacts don't actually need a reference.

Fix this by just falling back to the name-version if there is no reference defined.

(I'd like to refactor this code a bit, and just use a UUID as the file name to avoid all this, but this felt like a smaller change and I'd like to cherry pick the fix.)

We currently throw an exception if a helm artifact doesn't have a
reference. We're only using that reference to generate a file name
to use when downloading the artifact; helm artifacts don't actually
need a reference.

Fix this by just falling back to the name-version if there is no
reference defined.
Copy link
Contributor

@maggieneterval maggieneterval left a comment

Choose a reason for hiding this comment

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

🔨 👨‍🍳 🍪 ⚓ 📜 ✔️ 🎉

Copy link
Contributor

@ethanfrogers ethanfrogers left a comment

Choose a reason for hiding this comment

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

LGTM!

@ezimanyi ezimanyi merged commit 746fa9c into spinnaker:master Oct 10, 2019
@ezimanyi ezimanyi deleted the fix-null-reference branch October 10, 2019 20:55
@ezimanyi
Copy link
Contributor Author

@spinnakerbot 🍒 ⛏️ 1.16

@spinnakerbot
Copy link
Contributor

Cherry pick failed: Command failed (cherry pick commit 746fa9c) with exit code 1:

error: could not apply 746fa9c... fix(helm): Fix baking of helm artifacts (#442)
hint: after resolving the conflicts, mark the corrected paths
hint: with 'git add <paths>' or 'git rm <paths>'
hint: and commit the result with 'git commit'

@ezimanyi
Copy link
Contributor Author

Manual 🍒 ⛏️ to 1.16 in #443 .

louisjimenez pushed a commit that referenced this pull request Oct 10, 2019
We currently throw an exception if a helm artifact doesn't have a
reference. We're only using that reference to generate a file name
to use when downloading the artifact; helm artifacts don't actually
need a reference.

Fix this by just falling back to the name-version if there is no
reference defined.
opsmxdemo pushed a commit to OpsMx/rosco that referenced this pull request Sep 15, 2020
We currently throw an exception if a helm artifact doesn't have a
reference. We're only using that reference to generate a file name
to use when downloading the artifact; helm artifacts don't actually
need a reference.

Fix this by just falling back to the name-version if there is no
reference defined.
opsmxdemo pushed a commit to OpsMx/rosco that referenced this pull request Sep 16, 2020
We currently throw an exception if a helm artifact doesn't have a
reference. We're only using that reference to generate a file name
to use when downloading the artifact; helm artifacts don't actually
need a reference.

Fix this by just falling back to the name-version if there is no
reference defined.
sitay93 pushed a commit to spotinst/spinnaker-rosco that referenced this pull request Jun 27, 2023
We currently throw an exception if a helm artifact doesn't have a
reference. We're only using that reference to generate a file name
to use when downloading the artifact; helm artifacts don't actually
need a reference.

Fix this by just falling back to the name-version if there is no
reference defined.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
4 participants