feat: add common logic for supporting universe domain#621
Conversation
|
🤖 I detect that the PR title and the commit message differ and there's only one commit. To use the PR title for the commit history, you can use Github's automerge feature with squashing, or use -- conventional-commit-lint bot |
…on-api-core into add-helpers-for-universe
…on-api-core into add-helpers-for-universe
vchudnov-g
left a comment
There was a problem hiding this comment.
Looking good, but I left some comments.
One other comment: do you want to call this module universe_helpers?
from google.api_core import universe_helpers
universe_helpers.get_universe_domain()
universe_helpers.compare_universes()
I wonder whether there are better names for this module and methods.
Maybe universe_domain.determine() and universe_domain.compare()? Or maybe we need slightly more explicit names than that (universe.determine_domain(), universe.compare_domains()?)
(Feel free to push back)
…on-api-core into add-helpers-for-universe
…on-api-core into add-helpers-for-universe
vchudnov-g
left a comment
There was a problem hiding this comment.
Could you either use or remove the mTLS_Universe_Error declaration from this PR?. Right now it's not being used anywhere.
Aside from that, just a minor (non-blocking) testing suggestion.
| assert str(excinfo.value).find(fake_domain) >= 0 | ||
|
|
||
| with pytest.raises(universe.UniverseMismatchError) as excinfo: | ||
| universe.compare_domains(fake_domain, _Fake_Credentials()) |
There was a problem hiding this comment.
These won't fit in one line? Darn.
Fixes #624 🦕