Rename env var to BUILDKITE_STEP_CANCEL_FORCE_GRACE_PERIOD_SECONDS#3087
Rename env var to BUILDKITE_STEP_CANCEL_FORCE_GRACE_PERIOD_SECONDS#3087
BUILDKITE_STEP_CANCEL_FORCE_GRACE_PERIOD_SECONDS#3087Conversation
mitchbne
changed the title
mhaylock
left a comment
mhaylock
left a comment
There was a problem hiding this comment.
I think this is a good improvement (I did suggest it 😅).
My only hesitation is around whether its aligned with the rest of the agent that the ENV var name doesn't match the flag name 1:1.
clicommand/step_cancel.go
Outdated
| Value: defaultCancelGracePeriod, | ||
| Usage: "The number of seconds to wait for agents to finish uploading artifacts before transitioning unfinished jobs to a canceled state. ′--force′ must also be supplied for this to take affect", | ||
| EnvVar: "BUILDKITE_FORCE_GRACE_PERIOD_SECONDS,BUILDKITE_CANCEL_GRACE_PERIOD", | ||
| EnvVar: "BUILDKITE_FORCE_CANCEL_GRACE_PERIOD_SECONDS,BUILDKITE_CANCEL_GRACE_PERIOD", |
There was a problem hiding this comment.
@DrJosh9000, @wolfeidau is it ok that this environment variable doesn't 1:1 match the flag? It feels a bit redundant to include the work "cancel" in the flag name - but fairly important to have it in the global ENV var name.
There was a problem hiding this comment.
We have some similar differences in other flags such as https://github.com/buildkite/agent/blob/main/clicommand/tool_sign.go#L123
Env vars are a global space so prefixing seems legit to me.
There was a problem hiding this comment.
I guess a legitimate prefix (that matches the command name) would technically be BUILDKITE_STEP_CANCEL_FORCE_GRACE_PERIOD_SECONDS.
There was a problem hiding this comment.
I think it's good - it's clear that this env var goes with this flag, and the flag belongs in the context of the command but the env var stands alone.
mitchbne
force-pushed
the
rename-env-var-step-cancel-grace-period
branch
from
9786332 to
7f81f58
Compare
mitchbne
changed the title
BUILDKITE_STEP_CANCEL_FORCE_GRACE_PERIOD_SECONDS
| Value: defaultCancelGracePeriod, | ||
| Usage: "The number of seconds to wait for agents to finish uploading artifacts before transitioning unfinished jobs to a canceled state. ′--force′ must also be supplied for this to take affect", | ||
| EnvVar: "BUILDKITE_FORCE_GRACE_PERIOD_SECONDS,BUILDKITE_CANCEL_GRACE_PERIOD", | ||
| EnvVar: "BUILDKITE_STEP_CANCEL_FORCE_GRACE_PERIOD_SECONDS,BUILDKITE_CANCEL_GRACE_PERIOD", |
4 participants
Description
Meant to include a3b8339 in #3084 but messed up the rebase.
BUILDKITE_STEP_CANCEL_FORCE_GRACE_PERIOD_SECONDSfeels like a better name for this env variable.Testing
go test ./...). Buildkite employees may check this if the pipeline has run automatically.go fmt ./...)