-
Notifications
You must be signed in to change notification settings - Fork 2k
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
jobspec: fixup vault_grace deprecation #7310
Conversation
Followup to #7170 - Moved canonicalization of VaultGrace back into `api/` package. - Fixed tests. - Made docs styling consistent.
//COMPAT(0.12) VaultGrace is deprecated and unused as of Vault 0.5 | ||
if tmpl.VaultGrace == nil { | ||
tmpl.VaultGrace = timeToPtr(0) | ||
} |
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.
This removes the implicit parsing (sets VaultGrace to 0 whether it is set or unset) that the original contributer had added. Given the error message in structs.go, this seems correct just checking.
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.
Great catch! This is intentional. We need to canonicalize it here because jobspec parsing is skipped when submitting JSON jobs directly to the api.
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.
This lgtm
Failed tests are known flaky and unrelated (although the stream limits is one I need to fix today!) |
I'm going to lock this pull request because it has been closed for 120 days ⏳. This helps our maintainers find and focus on the active contributions. |
Followup to #7170
api/
package.