Feat: Add universe domain #3087
Conversation
product-auto-label
bot
added
size: l
There was a problem hiding this comment.
Two things:
-
testing appears to be missing is something that exercises the invalid combination of mismatched universe in credentials and service option.
-
ensure that the host/endpoint functions properly when issuing rpcs, though this may make more sense out of band as a smoke test until we have a better way of doing universe integration testing.
| } | ||
| } else { | ||
| // This calls the backend without the retry wrapper so first validate the universe domain. |
There was a problem hiding this comment.
for the purposes of standardizing the pattern here, is it worth using validateAndRunWithRetries here, but passing in a static retry config that only allows a single invocation rather than using getOptions().getRetrySettings()?
| } | ||
| }); | ||
| return new PageImpl<>(new JobPageFetcher(serviceOptions, cursor, optionsMap), cursor, jobs); | ||
| } catch (IllegalArgumentException | IOException e) { |
There was a problem hiding this comment.
Do we need to catch/translate the potential RetryHelper.RetryHelperException (even though we didn't appear to previously)?
This PR adds the ability to set the universe domain and validate that the parameters are correct prior to each RPC. This is done by wrapping the existing RPC with the provided validation ServiceOption method. The PR does not change existing functionality of BigQuery and all existing unit and integration tests passes for the default universe domain. Manual testing was performed on a non-default universe domain. However, IT will require additional work (as it require additional credential setup) and will be included in a future PR.
Thank you for opening a Pull Request! Before submitting your PR, there are a few things you can do to make sure it goes smoothly:
Fixes #3088 ☕️