bigtable_gc_policy: Adding support for duration lower than day #7879
Conversation
Signed-off-by: Kevin Labesse <kevin@labesse.me>
ghost
requested a review
from 
|
So, the existing tests pass using the seconds field, but using the |
| @@ -257,3 +267,17 @@ func generateBigtableGCPolicy(d *schema.ResourceData) (bigtable.GCPolicy, error) | |||
|
|
|||
| return policies[0], nil | |||
| } | |||
|
|
|||
| func getMaxAgeDuration(values map[string]interface{}) (time.Duration, error) { | |||
| d, ok := values["seconds"] | |||
There was a problem hiding this comment.
Terraform will default the value of seconds to 0 if it is unset (because int is a primitive value I believe). This prevents us from telling if seconds is set to the value 0, or unset. ok will always be true because of this, causing this code to return a duration of 0 seconds if days is specified.
Because 0 is an invalid duration, we should probably assume that a value of 0 is equivalent to seconds not being set.
On that note, we should enforce validation on the seconds field to make sure users set a positive int value. This could look like ValidateFunc: validation.IntAtLeast(1000) or some similar value
There was a problem hiding this comment.
I've change the type to string and use time.ParseDuration, and set a validation function
There was a problem hiding this comment.
Ah, that won't quite work either. Changing the type of the days field will cause existing user configs to stop working, which would be a breaking change.
I think what we want is to set them back to ints, set the ValidateFunc on the seconds field, to make sure anyone who specifies it enters a number greater than 0. Then we can assume that a 0 value is the same as unset, and use the value for days instead.
There was a problem hiding this comment.
Sorry, I'm dumb ... furthermore I've update only the half of the reference to second ...
Anyway, days stay a int and the new field duration accept a valid duration (in string). If duration is set use it, otherwise fallback on days
There was a problem hiding this comment.
Looks like validation fails:
1 error occurred:
* resource google_bigtable_gc_policy: ExactlyOneOf: max_age configuration
block reference (max_age.0.days) can only be used with TypeList and MaxItems:
I think that the ExactlyOneOf needs to be on the days block rather than the max_age block
|
I'm going to lock this issue because it has been closed for 30 days ⏳. This helps our maintainers find and focus on the active issues. If you feel this issue should be reopened, we encourage creating a new issue linking back to this one for added context. If you feel I made an error 🤖 🙉 , please reach out to my human friends 👉 hashibot-feedback@hashicorp.com. Thanks! |
ghost
locked as resolved and limited conversation to collaborators
Allow GC policy to have a duration lower than a day, bigtable supports GC policies down to the hour. In case one day the support go to minutes/seconds the code will be already good