Skip to content
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

Throw error on unsupported continents #89

Merged
merged 2 commits into from
Sep 12, 2024
Merged

Throw error on unsupported continents #89

merged 2 commits into from
Sep 12, 2024

Conversation

viranch
Copy link
Contributor

@viranch viranch commented Sep 8, 2024

Fixes #88

@viranch viranch requested a review from a team September 8, 2024 21:23
Copy link
Contributor

@ross ross left a comment

Choose a reason for hiding this comment

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

Generally OK with this, but potential support or warn type option inline.

):
msg = f'unsupported continent code {geo} in {record.fqdn}'
# no workable fallbacks so straight error
raise SupportsException(f'{self.id}: {msg}')
Copy link
Contributor

Choose a reason for hiding this comment

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

Only potential fallback type behavior I can think of would be ignoring the unsupported continent and letting things match whatever the other rules would do, presumably that'd be a default in the specific case here.

That'd probably require removing the code if there are other valid ones or the whole rule if nothing is valid with the warn if strict stuff. Not sure if that's worth it, but it would at least allow other providers to still target the unsupported stuff while managing things in parallel to NS1 rather than preventing them from being used.

Up to you.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yeah, and if the whole rule to be removed points to an exclusively used pool, that would have to be determined and removed too.

Not opposed to it but also don't have enough incentive to implement it that way (yet).

@viranch viranch merged commit 5070452 into main Sep 12, 2024
7 checks passed
@viranch viranch deleted the invalid-continent branch September 12, 2024 00:26
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.

Invalid geos don't throw error
2 participants