Repository navigation
Conversation
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## master #40672 +/- ##
=============================================
- Coverage 56.16% 0 -56.17%
=============================================
Files 1124 0 -1124
Lines 178336 0 -178336
Branches 1489 0 -1489
=============================================
- Hits 100170 0 -100170
+ Misses 75645 0 -75645
+ Partials 2521 0 -2521
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Harness. 🚀 New features to boost your workflow:
|
apiRootUrl defaulted to https://dataflow.googleapis.com/, so DataflowTransport always called setRootUrl, which stopped google-api-client from honoring GOOGLE_CLOUD_UNIVERSE_DOMAIN. apiRootUrl now has no default, and the root URL and service path are set only when apiRootUrl or dataflowEndpoint is given.
|
/gemini review |
There was a problem hiding this comment.
📋 Review Summary
This PR updates the Java Dataflow runner to defer Dataflow API root URL resolution to the Google API client library when neither --apiRootUrl nor an absolute --dataflowEndpoint is explicitly configured, enabling GOOGLE_CLOUD_UNIVERSE_DOMAIN support. The implementation, nullability annotations, documentation updates, and unit test coverage in DataflowTransportTest are clean and well-structured.
🔍 General Feedback
- Removing
@Default.String(Dataflow.DEFAULT_ROOT_URL)fromDataflowPipelineDebugOptions#getApiRootUrl()and conditionally callingbuilder.setRootUrl(...)inDataflowTransportaccurately avoids settingisUserConfiguredEndpoint = trueinAbstractGoogleClient.Builder, preserving universe domain resolution. - The subtle behavior change for callers inspecting
getApiRootUrl()(now returningnullwhen unset) is appropriately annotated with@Nullableand documented inCHANGES.md.
💡 Tip: Comment /gemini review on this pull request to trigger a new Gemini code review.
|
Assigning reviewers: R: @chamikaramj added as fallback since no labels match configuration Note: If you would like to opt out of this review, comment Available commands:
The PR bot will only process comments in the main thread (not review comments). |
Part of #40569, Builds upon #40670 (needs universe domain support from googleapis/google-api-java-client#2435)
Problem
The Java Dataflow runner always sends requests to
https://dataflow.googleapis.com/, even whenGOOGLE_CLOUD_UNIVERSE_DOMAINpoints to another universe.apiRootUrldefaults tohttps://dataflow.googleapis.com/, andDataflowTransportalways callsDataflow.Builder#setRootUrl. In google-api-client, callingsetRootUrl(even with the default URL) marks the endpoint as user-configured, so the library never builds thehttps://dataflow.<universe>/endpoint.Changes
apiRootUrlno longer has a default.DataflowTransportsets the root URL and service path only whenapiRootUrlordataflowEndpointis set. Otherwise the client library resolves the endpoint, honoringGOOGLE_CLOUD_UNIVERSE_DOMAINand falling back to the public URL.dataflowEndpointapiRootUrlapiRootUrl(+ path)dataflowEndpointBreaking change
DataflowPipelineDebugOptions.getApiRootUrl()now returnsnullby default instead ofhttps://dataflow.googleapis.com/.