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.

Sign up for GitHub

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

Jump to bottom

Deal with the change in helm's --timeout flag #39

Closed
ErinCall opened this issue Dec 27, 2019 · 1 comment · Fixed by #47
Closed

Deal with the change in helm's --timeout flag #39

ErinCall opened this issue Dec 27, 2019 · 1 comment · Fixed by #47
Assignees
@ErinCall
Labels
enhancement New feature or request
Milestone
Version 1.0

Comments

@ErinCall
Copy link
Contributor

The problem I'm trying to solve:
helm2's --timeout flag specified a number of seconds for the timeout. In helm3 it uses a string formatted for golang's ParseDuration function, e.g. "200s".

If someone is upgrading from drone-helm and they have a timeout: 200 in their settings, it won't do what they expect. I'm not sure whether helm will just exit with an error or set the timeout to 0, but either way the deployment won't succeed.

How I imagine it working:
I see two options:

  1. Say "update your timeout setting" in the "Upgrading from drone-helm" section of the readme
  2. If we see a bare number in cfg.Timeout, append an s so it means the same thing it did in helm2
@ErinCall ErinCall added the enhancement New feature or request label Dec 27, 2019
@ErinCall ErinCall added this to the Version 1.0 milestone Dec 27, 2019
@josmo
Copy link
Member

josmo commented Dec 27, 2019

I personally say lets go with option 2, it will make converting simpler for folks

@ErinCall ErinCall self-assigned this Dec 27, 2019
ErinCall added a commit that referenced this issue Dec 28, 2019
@ErinCall
Shim bare numbers into duration strings [#39]
d5a5959 
Helm2's --timeout took a number of seconds, rather than the
ParseDuration-compatible string that helm3 uses. For backward-
compatibility, update a bare number into a duration string.
@ErinCall ErinCall mentioned this issue Dec 28, 2019
Merged
4 tasks
@josmo josmo closed this as completed in #47 Dec 28, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees

@ErinCall ErinCall

Labels
enhancement New feature or request
Projects
None yet
Milestone
Version 1.0
Development

Successfully merging a pull request may close this issue.

Shim bare numbers into duration strings pelotech/drone-helm3
2 participants
@ErinCall @josmo