[BEAM-1071] Allow for streaming tables with CREATE_NEVER disposition by sammcveety · Pull Request #1590 · apache/beam

sammcveety · 2016-12-13T02:48:22Z

This turned out a bit more involved than I'd hoped.

asfbot · 2016-12-13T04:48:13Z

Refer to this link for build results (access rights to CI server needed):
https://builds.apache.org/job/beam_PreCommit_Java_MavenInstall/5809/

Build result: FAILURE

[...truncated 6618 lines...]No artifacts from beam_PreCommit_Java_MavenInstall » Apache Beam :: Runners :: Apex #5808 to compare, so performing full copy of artifacts[JENKINS] Archiving /home/jenkins/jenkins-slave/workspace/beam_PreCommit_Java_MavenInstall/sdks/java/io/jms/pom.xml to org.apache.beam/beam-sdks-java-io-jms/0.4.0-incubating-SNAPSHOT/beam-sdks-java-io-jms-0.4.0-incubating-SNAPSHOT.pomNo artifacts from beam_PreCommit_Java_MavenInstall » Apache Beam :: SDKs :: Java :: IO :: JMS #5808 to compare, so performing full copy of artifacts[JENKINS] Archiving /home/jenkins/jenkins-slave/workspace/beam_PreCommit_Java_MavenInstall/runners/flink/pom.xml to org.apache.beam/beam-runners-flink-parent/0.4.0-incubating-SNAPSHOT/beam-runners-flink-parent-0.4.0-incubating-SNAPSHOT.pomNo artifacts from beam_PreCommit_Java_MavenInstall » Apache Beam :: Runners :: Flink #5808 to compare, so performing full copy of artifacts[JENKINS] Archiving /home/jenkins/jenkins-slave/workspace/beam_PreCommit_Java_MavenInstall/sdks/java/io/mongodb/pom.xml to org.apache.beam/beam-sdks-java-io-mongodb/0.4.0-incubating-SNAPSHOT/beam-sdks-java-io-mongodb-0.4.0-incubating-SNAPSHOT.pomNo artifacts from beam_PreCommit_Java_MavenInstall » Apache Beam :: SDKs :: Java :: IO :: MongoDB #5808 to compare, so performing full copy of artifacts[JENKINS] Archiving /home/jenkins/jenkins-slave/workspace/beam_PreCommit_Java_MavenInstall/sdks/java/extensions/sorter/pom.xml to org.apache.beam/beam-sdks-java-extensions-sorter/0.4.0-incubating-SNAPSHOT/beam-sdks-java-extensions-sorter-0.4.0-incubating-SNAPSHOT.pomNo artifacts from beam_PreCommit_Java_MavenInstall » Apache Beam :: SDKs :: Java :: Extensions :: Sorter #5808 to compare, so performing full copy of artifacts[JENKINS] Archiving /home/jenkins/jenkins-slave/workspace/beam_PreCommit_Java_MavenInstall/sdks/pom.xml to org.apache.beam/beam-sdks-parent/0.4.0-incubating-SNAPSHOT/beam-sdks-parent-0.4.0-incubating-SNAPSHOT.pomNo artifacts from beam_PreCommit_Java_MavenInstall » Apache Beam :: SDKs #5808 to compare, so performing full copy of artifacts[JENKINS] Archiving /home/jenkins/jenkins-slave/workspace/beam_PreCommit_Java_MavenInstall/sdks/java/extensions/join-library/pom.xml to org.apache.beam/beam-sdks-java-extensions-join-library/0.4.0-incubating-SNAPSHOT/beam-sdks-java-extensions-join-library-0.4.0-incubating-SNAPSHOT.pomNo artifacts from beam_PreCommit_Java_MavenInstall » Apache Beam :: SDKs :: Java :: Extensions :: Join library #5808 to compare, so performing full copy of artifacts[JENKINS] Archiving /home/jenkins/jenkins-slave/workspace/beam_PreCommit_Java_MavenInstall/runners/pom.xml to org.apache.beam/beam-runners-parent/0.4.0-incubating-SNAPSHOT/beam-runners-parent-0.4.0-incubating-SNAPSHOT.pomNo artifacts from beam_PreCommit_Java_MavenInstall » Apache Beam :: Runners #5808 to compare, so performing full copy of artifacts[JENKINS] Archiving /home/jenkins/jenkins-slave/workspace/beam_PreCommit_Java_MavenInstall/sdks/java/io/jdbc/pom.xml to org.apache.beam/beam-sdks-java-io-jdbc/0.4.0-incubating-SNAPSHOT/beam-sdks-java-io-jdbc-0.4.0-incubating-SNAPSHOT.pomNo artifacts from beam_PreCommit_Java_MavenInstall » Apache Beam :: SDKs :: Java :: IO :: JDBC #5808 to compare, so performing full copy of artifacts[JENKINS] Archiving /home/jenkins/jenkins-slave/workspace/beam_PreCommit_Java_MavenInstall/sdks/java/extensions/pom.xml to org.apache.beam/beam-sdks-java-extensions-parent/0.4.0-incubating-SNAPSHOT/beam-sdks-java-extensions-parent-0.4.0-incubating-SNAPSHOT.pomNo artifacts from beam_PreCommit_Java_MavenInstall » Apache Beam :: SDKs :: Java :: Extensions #5808 to compare, so performing full copy of artifacts[JENKINS] Archiving /home/jenkins/jenkins-slave/workspace/beam_PreCommit_Java_MavenInstall/examples/pom.xml to org.apache.beam/beam-examples-parent/0.4.0-incubating-SNAPSHOT/beam-examples-parent-0.4.0-incubating-SNAPSHOT.pomNo artifacts from beam_PreCommit_Java_MavenInstall » Apache Beam :: Examples #5808 to compare, so performing full copy of artifactschannel stoppedSetting status of 7624040 to FAILURE with url https://builds.apache.org/job/beam_PreCommit_Java_MavenInstall/5809/ and message: 'Build finished. 'Using context: Jenkins: Maven clean install
--none--

sammcveety · 2016-12-13T14:20:40Z

retest this please

sammcveety · 2016-12-13T14:34:36Z

Failure appears unrelated:

2016-12-13T13:55:36.768 [ERROR] Failed to execute goal org.apache.maven.plugins:maven-archetype-plugin:2.4:jar (default-jar) on project beam-sdks-java-maven-archetypes-starter: Execution default-jar of goal org.apache.maven.plugins:maven-archetype-plugin:2.4:jar failed: Plugin org.apache.maven.plugins:maven-archetype-plugin:2.4 or one of its dependencies could not be resolved: Failed to collect dependencies at org.apache.maven.plugins:maven-archetype-plugin:jar:2.4 -> org.apache.maven.shared:maven-invoker:jar:2.2: Failed to read artifact descriptor for org.apache.maven.shared:maven-invoker:jar:2.2: Could not transfer artifact org.apache.maven.shared:maven-invoker:pom:2.2 from/to central (https://repo.maven.apache.org/maven2): Access denied to: https://repo.maven.apache.org/maven2/org/apache/maven/shared/maven-invoker/2.2/maven-invoker-2.2.pom , ReasonPhrase:Forbidden. -> [Help 1]

asfbot · 2016-12-13T14:35:00Z

Refer to this link for build results (access rights to CI server needed):
https://builds.apache.org/job/beam_PreCommit_Java_MavenInstall/5823/

Failed Tests: 1

beam_PreCommit_Java_MavenInstall/org.apache.beam:beam-examples-java: 1

org.apache.beam.examples.WordCountIT.testE2EWordCount

--none--

sammcveety · 2016-12-13T14:35:57Z

retest this please

asfbot · 2016-12-13T15:13:42Z

Refer to this link for build results (access rights to CI server needed):
https://builds.apache.org/job/beam_PreCommit_Java_MavenInstall/5825/

Failed Tests: 1

beam_PreCommit_Java_MavenInstall/org.apache.beam:beam-examples-java: 1

org.apache.beam.examples.WordCountIT.testE2EWordCount

--none--

asfbot · 2016-12-13T15:28:38Z

Refer to this link for build results (access rights to CI server needed):
https://builds.apache.org/job/beam_PreCommit_Java_MavenInstall/5826/

Failed Tests: 1

beam_PreCommit_Java_MavenInstall/org.apache.beam:beam-examples-java: 1

org.apache.beam.examples.WordCountIT.testE2EWordCount

--none--

dhalperi

Not sure this is really catching new error cases well. I added a few suggestive comments of various constructions I think might slip through validation now.

dhalperi · 2016-12-13T20:51:49Z

...a/io/google-cloud-platform/src/main/java/org/apache/beam/sdk/io/gcp/bigquery/BigQueryIO.java

        throws IOException {
      TableReference tableReference = parseTableSpec(tableSpec);
-      if (!createdTables.contains(tableSpec)) {
+      if (!createdTables.contains(tableSpec) && jsonTableSchema != null) {


Instead of using jsonTableSchema != null can we check CreateDisposition == NEVER?

I'd need to pass the parameter through and store, we're good with that?

dhalperi · 2016-12-13T21:00:55Z

...a/io/google-cloud-platform/src/main/java/org/apache/beam/sdk/io/gcp/bigquery/BigQueryIO.java

+                createDisposition != CreateDisposition.CREATE_NEVER,
+                "CreateDisposition.CREATE_NEVER is not supported when using a tablespec"
+                + " function.");
+          }


Can we check the dual of this where it is appropriate? : If the schema is null, then CreateDisposition == CreateNEVER

This will break when combined with my PR. What's the rationale for this PR?

The rationale is to be able to run a streaming pipeline against an already-created table without needing to specify the schema. Very useful for some templates.

Sounds like Reuven is now ok with this change. looking

asfbot · 2016-12-17T22:53:29Z

Refer to this link for build results (access rights to CI server needed):
https://builds.apache.org/job/beam_PreCommit_Java_MavenInstall/6056/
--none--

sammcveety · 2016-12-17T22:55:47Z

Added some more testing and addressed comments, please take another look.

reuvenlax · 2016-12-18T11:36:58Z

You're going to have to merge with the current version of the code

sammcveety · 2016-12-19T00:54:31Z

@reuvenlax could you be more specific? Github says there are no merge conflicts, and all tests pass.

dhalperi · 2016-12-19T16:52:20Z

.../google-cloud-platform/src/test/java/org/apache/beam/sdk/io/gcp/bigquery/BigQueryIOTest.java

+    TableReference tableRef = new TableReference();
+    tableRef.setDatasetId("dataset");
+    tableRef.setTableId("sometable");
+


Create.<TableRow>of()?

This creates a bounded collection, which fails the UNBOUNDED conditionals. This raises a few possibilities:

Should CreateUnbounded be a shorthand we have?

Should these conditionals check options.isStreaming() (guessing not, because that's less clean with respect to the model)

No, I was just wrong. You were right.

reuvenlax · 2016-12-19T17:27:58Z

@sammcveety that's because your branch doesn't include latest changes in this file - specifically the testing ones. You will have to fetch changes and rebase to origin/master to sync - then you'll have to resolve conflicts. In addition to conflicts, getOrCreateTable will have to be fixed, as it no longer supports create_never.

sammcveety · 2017-01-10T19:30:40Z

retest this please

asfbot · 2017-01-10T19:57:25Z

Refer to this link for build results (access rights to CI server needed):
https://builds.apache.org/job/beam_PreCommit_Java_MavenInstall/6486/
--none--

sammcveety · 2017-01-10T21:02:04Z

@reuvenlax I retested this and it is running against head. I believe it to be ready for another review by @dhalperi .

dhalperi

LGTM.

I can make the minor fix in merging .

dhalperi · 2017-01-24T22:34:47Z

...a/io/google-cloud-platform/src/main/java/org/apache/beam/sdk/io/gcp/bigquery/BigQueryIO.java

      TableReference tableReference = parseTableSpec(tableSpec);
-      if (!createdTables.contains(tableSpec)) {
+      if (!createdTables.contains(tableSpec)
+          && createDisposition != createDisposition.CREATE_NEVER) {


swap check order? createdTables is a concurrent data structure.

Allow for BQ tables never

7624040

Fix conditional

3505cf8

dhalperi requested changes Dec 13, 2016

View reviewed changes

Fixups

c07bfc9

dhalperi reviewed Dec 19, 2016

View reviewed changes

dhalperi reviewed Jan 24, 2017

View reviewed changes

asfgit closed this in f2389ab Jan 24, 2017

Conversation

sammcveety commented Dec 13, 2016 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

asfbot commented Dec 13, 2016

Build result: FAILURE

Uh oh!

sammcveety commented Dec 13, 2016

Uh oh!

sammcveety commented Dec 13, 2016

Uh oh!

asfbot commented Dec 13, 2016

Failed Tests: 1

beam_PreCommit_Java_MavenInstall/org.apache.beam:beam-examples-java: 1

Uh oh!

sammcveety commented Dec 13, 2016

Uh oh!

asfbot commented Dec 13, 2016

Failed Tests: 1

beam_PreCommit_Java_MavenInstall/org.apache.beam:beam-examples-java: 1

Uh oh!

asfbot commented Dec 13, 2016

Failed Tests: 1

beam_PreCommit_Java_MavenInstall/org.apache.beam:beam-examples-java: 1

Uh oh!

dhalperi left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

asfbot commented Dec 17, 2016

Uh oh!

sammcveety commented Dec 17, 2016

Uh oh!

reuvenlax commented Dec 18, 2016

Uh oh!

sammcveety commented Dec 19, 2016

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

reuvenlax commented Dec 19, 2016

Uh oh!

sammcveety commented Jan 10, 2017

Uh oh!

asfbot commented Jan 10, 2017

Uh oh!

sammcveety commented Jan 10, 2017

Uh oh!

dhalperi left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

sammcveety commented Dec 13, 2016 •

edited

Loading