kennknowles commented on code in PR #28621:
URL: https://github.com/apache/beam/pull/28621#discussion_r1368991768
##########
runners/google-cloud-dataflow-java/src/test/java/org/apache/beam/runners/dataflow/DataflowRunnerTest.java:
##########
@@ -824,6 +840,24 @@ public void testUploadGraph() throws IOException {
.startsWith("gs://valid-bucket/temp/staging/dataflow_graph"));
}
+ /** Test for automatically using upload_graph when the job graph is too
large (>10MB). */
+ @Test
+ public void testUploadGraphWithAutoUpload() throws IOException {
+ DataflowPipelineOptions options = buildPipelineOptions();
+ Pipeline p = buildDataflowPipelineWithLargeGraph(options);
+ p.run();
+
+ ArgumentCaptor<Job> jobCaptor = ArgumentCaptor.forClass(Job.class);
+ Mockito.verify(mockJobs).create(eq(PROJECT_ID), eq(REGION_ID),
jobCaptor.capture());
+ assertValidJob(jobCaptor.getValue());
+ assertTrue(jobCaptor.getValue().getSteps().isEmpty());
+ assertTrue(
+ jobCaptor
+ .getValue()
+ .getStepsLocation()
+ .startsWith("gs://valid-bucket/temp/staging/dataflow_graph"));
Review Comment:
nit: here and in the other tests `gs://valid-bucket/temp/staging/` should be
part of the test case. It is not clear from the test if it is:
1. Part of the spec for this feature
2. A constant behavior of the code under test (in which case it should be a
reference to that)
3. A value pass in from global test set up (this is an anti-pattern)
Just noting this - the whole file has this problem a lot and you don't have
to fix it.
##########
CHANGES.md:
##########
@@ -72,6 +72,8 @@ should handle this.
([#25252](https://github.com/apache/beam/issues/25252)).
## Breaking Changes
+* `upload_graph` is not needed since the graph now is uploaded automatically
when it is larger than 10MB for Java SDK
([PR#28621](https://github.com/apache/beam/pull/28621).
Review Comment:
It won't break anyone though. it is more like a new cool feature.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
