go-gandi / terraform-provider-gandi

Terraform provider for the Gandi Domain services
Mozilla Public License 2.0
153 stars 46 forks source link

livedns: do not require TTL when specifying a domain #90

Closed vincentbernat closed 2 years ago

vincentbernat commented 2 years ago

It is not possible currently to query the current TTL, making it difficult to import and manipulate an existing domain. Make the TTL optional solves this issue. Also update the documentation: the TTL is for the SOA, this is not the default one.

Also, set a default value for autosnapshots (matching the current default value). Otherwise, we have a change each time the resource is refreshed.

Fix #55 Fix #57

nlewo commented 2 years ago

Actually, this TTL parameter should not be here: it is not useful for the end user, only for some internal Gandi stuffs.

I'm fine by moving it as optional but it would be nice to deprecate it. Could you add a deprecate attribute, such as https://github.com/go-gandi/terraform-provider-gandi/commit/cf86b426838002e48773178d567a652af2eece66#diff-e7ccf8727112e38d52554f35fdfc288eafb6c6de8f4a8bc7dfb6d863120f420cR31 ?

Also, if you rebase onto master, the CI should be green.

vincentbernat commented 2 years ago

Done. I didn't regenerate the documentation. I expect this to be done before release. Alternatively, maybe a target in the Makefile would be useful (but this requires additional deps in go.mod).

vincentbernat commented 2 years ago

Rebasing does not seem enough.

nlewo commented 2 years ago

Rebasing does not seem enough

Arf, i should have test my CI fix attempt a bit more :/ BTW, the failure is not due to this patch.

I have plan to regenerate the doc for the release (thanks for the reminder).

I don't think adding a dep is a big issue (especially since user use prebuilt binaries).

Thank you for these improvements!

robbyoconnor commented 2 years ago

@nlewo Good work!