Skip to content

Conversation

@lukecwik
Copy link
Member

Break out tests to Dataflow specific variants to make move of Dataflow pipeline runner to new maven module.

Also, swap usage of DataflowPipelineOptions for other options interfaces which were better suited. For example, creating PubsubOptions.

lukecwik added 10 commits March 29, 2016 10:52
This allows for moving the Dataflow specific portion of the test to the Dataflow runner maven module.
This allows for moving the Dataflow specific portion of the testing to the Dataflow runner maven module.
This prevents moving the Dataflow runner code to its own separate maven module.
…rface

This prevents moving DataflowPipelineOptions to the Dataflow runner maven module.
This prevents moving DataflowPipelineOptions into a Dataflow runner maven module.
This prevents moving DataflowPipelineOptions into a separate maven module.
Removed DatastoreIO dependence on numWorkers from DataflowPipelineWorkerHarnessOptions.
This prevented moving DataflowPipelineOptions to a separate maven module.
This prevents moving DataflowPipelineRunner and subclasses to runner specific maven module.
This prevents moving DataflowPipelineOptions to its own runner specifc maven module.
@lukecwik
Copy link
Member Author

R: @dhalperi

@dhalperi
Copy link
Contributor

Random attempt:

@dhalperi
Copy link
Contributor

Jenkins, retest this please.

@lukecwik
Copy link
Member Author

Merged in apache/master to force re-run of tests

* Properties that can be set when using Pubsub with the Dataflow SDK.
*/
@Description("Options that are used to configure BigQuery. See "
+ "https://cloud.google.com/bigquery/what-is-bigquery for details on BigQuery.")
Copy link
Contributor

Choose a reason for hiding this comment

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

description is off

Copy link
Member Author

Choose a reason for hiding this comment

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

Done

@davorbonaci
Copy link
Member

R: @davorbonaci
(I'd also like to take a peek.)

/**
* Helpers for cloud communication.
*/
public class DataflowTransport {
Copy link
Contributor

Choose a reason for hiding this comment

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

Dataflow or Gcp?

I assume these are used outside of Dataflow Service, e.g., PubSubIO on Flink.

Copy link
Contributor

Choose a reason for hiding this comment

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

Oh I see you only moved Dataflow and Clouddebugger. Why only these?

Copy link
Contributor

Choose a reason for hiding this comment

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

never mind. resolved.

Copy link
Member Author

Choose a reason for hiding this comment

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

Yeah, I left all the GCP IO transports in Transport.java and moved out to the two clients which we use only with Dataflow. In the future, cloud debugger could move out to be generic for others as well.

@davorbonaci
Copy link
Member

No particular comments from me.

* {@link DataflowPipelineRunner} specific tests for TextIO Read and Write transforms.
*/
@RunWith(JUnit4.class)
public class DataflowTextIOTest {
Copy link
Member

Choose a reason for hiding this comment

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

Remove; duplicate exists elsewhere.

Copy link
Member

Choose a reason for hiding this comment

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

Retracting.

@davorbonaci
Copy link
Member

LGTM (self-merge).

@dhalperi
Copy link
Contributor

dhalperi commented Apr 6, 2016

@lukecwik bumping that you should self-merge this PR when ready.

@asfgit asfgit closed this in 6467afb Apr 7, 2016
@lukecwik lukecwik deleted the dataflow-runner-2 branch September 26, 2017 22:11
iemejia referenced this pull request in iemejia/beam Jan 12, 2018
mareksimunek pushed a commit to mareksimunek/beam that referenced this pull request May 9, 2018
apache#85 Move findbugs plugin execution to the process-classes phase
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.

3 participants