Skip to content

Catch Route53 Record Sets without TTL#7767

Merged
thrau merged 1 commit intolocalstack:masterfrom
quinnypig:patch-2
Mar 1, 2023
Merged

Catch Route53 Record Sets without TTL#7767
thrau merged 1 commit intolocalstack:masterfrom
quinnypig:patch-2

Conversation

@quinnypig
Copy link
Contributor

Alias records, for example, should explicitly not have a TTL set, as it's derived from the TTL of the target.

Alias records, for example, should explicitly not have a TTL set, as it's derived from the TTL of the target.
@quinnypig quinnypig requested a review from giograno as a code owner March 1, 2023 00:12
Copy link
Member

@giograno giograno left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for the contribution @quinnypig! 🙏

Comment on lines +41 to +43
if "TTL" in attrs:
if isinstance(attrs["TTL"], str):
attrs["TTL"] = int(attrs["TTL"])
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
if "TTL" in attrs:
if isinstance(attrs["TTL"], str):
attrs["TTL"] = int(attrs["TTL"])
if isinstance(ttl := attrs.get("TTL"), str):
attrs["TTL"] = int(ttl)

We could avoid the nested ifs with something like this.

@thrau thrau merged commit ba4ee37 into localstack:master Mar 1, 2023
@thrau
Copy link
Member

thrau commented Mar 1, 2023

looked safe to merge, we'll have to check what's going on with the CI with community PRs

@quinnypig quinnypig deleted the patch-2 branch March 1, 2023 19:22
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants