-
Notifications
You must be signed in to change notification settings - Fork 154
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
feat(workspace): add auto destroy activity duration #1377
Conversation
@@ -112,9 +116,29 @@ func resourceTFEWorkspace() *schema.Resource { | |||
|
|||
"auto_destroy_at": { | |||
Type: schema.TypeString, | |||
Computed: true, |
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.
Unfortunately most of the raw config/state checking in this PR is due to this value being changed to computed
. The main problem is insuring that unsetting auto_destroy_at
(when auto_destroy_activity_duration
is also unset) sends a null value to the TFC API.
bf992b3
to
c1c4cd0
Compare
c1c4cd0
to
6074f95
Compare
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.
Overall looking good so far! I'm going to take it for a test run and will report back.
Type: schema.TypeString, | ||
Optional: true, | ||
ConflictsWith: []string{"auto_destroy_at"}, | ||
ValidateFunc: validation.StringMatch(regexp.MustCompile(`^\d{1,5}[dh]$`), "must be 1-5 digits followed by d or h"), |
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.
should be
ValidateFunc: validation.StringMatch(regexp.MustCompile(`^\d{1,5}[dh]$`), "must be 1-5 digits followed by d or h"), | |
ValidateFunc: validation.StringMatch(regexp.MustCompile(`^\d{1,4}[dh]$`), "must be 1-4 digits followed by d or h"), |
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.
edge case but 0d
or 0h
seems to be a valid value but fails when trying to apply
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.
edge case but
0d
or0h
seems to be a valid value but fails when trying to apply
validation is mostly an additional nice-to-have to give the user early feedback. I'm fine with the logic here not needing to 100% match the business logic in the API, since it's an edge case
functionality looks good. tested and working as intended |
Description
Adds
auto_destroy_activity_duration
to the tfe_workspace resource.Requires #1378Remember to:
Testing plan
auto_destroy_activity_duration
External links
Include any links here that might be helpful for people reviewing your PR. If there are none, feel free to delete this section.
Output from acceptance tests
Please run applicable acceptance tests locally and include the output here. See testing.md to learn how to run acceptance tests.
If you are an external contributor, your contribution(s) will first be reviewed before running them against the project's CI pipeline.