Support DatabaseID in Datastore beam connector by pl04351820 · Pull Request #27987 · apache/beam

pl04351820 · 2023-08-14T01:11:14Z

[Feature] Support DatabaseID in Datastore beam connector

This allows users to explicit set DatabaseID in datastore beam connector for read and write.
Updated existing beam connector to explicitly set projectID and databaseID.
The Read option will access DatabaseID via option class defined in DatastoreV1.
The Write option will access DatabaseID via FirestoreOptions.

…tidb

github-actions · 2023-08-14T02:10:02Z

Assigning reviewers. If you would like to opt out of this review, comment assign to next reviewer:

R: @robertwb for label java.
R: @johnjcasey for label io.

Available commands:

stop reviewer notifications - opt out of the automated review tooling
remind me after tests pass - tag the comment author after tests pass
waiting on author - shift the attention set back to the author (any comment or push by the author will return the attention set to the reviewers)

The PR bot will only process comments in the main thread (not review comments).

bvolpato · 2023-08-14T12:32:23Z

Run Java_GCP_IO_Direct PreCommit

pl04351820 · 2023-08-14T15:00:24Z

Looks like the failing tests in Direct IO are coming from BigQuery and unrelated to the change:

[org.apache.beam.sdk.io.gcp.bigquery.BigQueryIOWriteTest.testWriteAvro[0]](https://ci-beam.apache.org/job/beam_PreCommit_Java_GCP_IO_Direct_Commit/3301/testReport/junit/org.apache.beam.sdk.io.gcp.bigquery/BigQueryIOWriteTest/testWriteAvro_0_/)
[org.apache.beam.sdk.io.gcp.bigquery.BigQueryIOWriteTest.testUpsertAndDeleteBeamRows[4]](https://ci-beam.apache.org/job/beam_PreCommit_Java_GCP_IO_Direct_Commit/3301/testReport/junit/org.apache.beam.sdk.io.gcp.bigquery/BigQueryIOWriteTest/testUpsertAndDeleteBeamRows_4_/)

...io/google-cloud-platform/src/main/java/org/apache/beam/sdk/io/gcp/datastore/DatastoreV1.java

johnjcasey · 2023-08-14T15:09:11Z

...io/google-cloud-platform/src/main/java/org/apache/beam/sdk/io/gcp/datastore/DatastoreV1.java

    public void startBundle(StartBundleContext c) {
-      datastore = datastoreFactory.getDatastore(c.getPipelineOptions(), projectId.get(), localhost);
+      // Read Firestore DatabaseID from FirestoreOptions.
+      String firestoreDatabaseId =


why are we passing the databaseId around, but querying for it from options here? I would think that it should be passed in from the constructor.

We hope users can run the datastore beam with flag for write. StartBundleContext.

"--firestoreDb=${firestoreDb}",

Similar example in Firestore:

beam/sdks/java/io/google-cloud-platform/src/main/java/org/apache/beam/sdk/io/gcp/firestore/FirestoreV1WriteFn.java

Line 206 in d2663f9

String databaseId = c.getPipelineOptions().as(FirestoreOptions.class).getFirestoreDb();

This doesn't track to for me. It looks like you pass the value around when constricting the IO, but then query from pipeline options here. That is very confusing for a user.

A user could do .withDatabaseId(ID), and that doesn't appear to do anything

Thanks for pointing this out! Now I see why this is confusing.

It's unexpected users can do DatabaseID in WriteFn. The only way that can pass DatabaseID is from FirestoreOptions. Removed withDatabaseId references for Write.

I think this is an antipattern. I don't think we should be passing in options via options. What if the user wanted to query from two databases in a single pipeline?

...io/google-cloud-platform/src/main/java/org/apache/beam/sdk/io/gcp/datastore/DatastoreV1.java

johnjcasey

We shouldn't have databaseID passed in as a pipeline option. It doesn't support reading from two databases in a single pipeline

…tidb

...io/google-cloud-platform/src/main/java/org/apache/beam/sdk/io/gcp/datastore/DatastoreV1.java

bvolpato

In general it LGTM -- my only concern is on the ValueProvider usage, as it might break and throw IllegalStateException when provided in runtime.

…tidb

pl04351820 · 2023-10-17T15:56:59Z

I run the failed tests locally and looks like they passed.

./gradlew integrationTest -p sdks/java/io/google-cloud-platform -PgcpProject={project-id} --tests=org.apache.beam.sdk.io.gcp.datastore.V1WriteIT.testE2EV1Write  --info --stacktrace

pl04351820 · 2023-10-17T15:57:27Z

Run Java_GCP_IO_Direct PreCommit

bvolpato · 2023-10-30T15:21:36Z

Run Java_GCP_IO_Direct PreCommit

bvolpato

LGTM, thanks!

This reverts commit 181296d.

) This reverts commit 181296d.

Sichen Liu and others added 12 commits August 4, 2023 12:35

Fix routing header issue in Firestore Beam connector

fca6bc1

Apply the lint

cf044d0

Merge branch 'main' of https://github.com/pl04351820/beam

606e2e2

Apply the lint fix

3aee7a7

Merge branch 'main' of https://github.com/pl04351820/beam

493b971

Support multi-db in datastore beam

f5b24af

Support multi-db in datastore beam

4e0e386

Merge branch 'multidb' of https://github.com/pl04351820/beam into mul…

d68d52e

…tidb

Merge branch 'multidb' of https://github.com/pl04351820/beam into mul…

a09a9c0

…tidb

Merge branch 'multidb' of https://github.com/pl04351820/beam into mul…

f18d5e1

…tidb

Merge branch 'multidb' of https://github.com/pl04351820/beam into mul…

cc6ae30

…tidb

Merge branch 'multidb' of https://github.com/pl04351820/beam into mul…

e5472bd

…tidb

github-actions bot added java io gcp labels Aug 14, 2023

pl04351820 added 3 commits August 13, 2023 18:16

Merge branch 'apache:master' into multidb

255d518

Merge branch 'apache:master' into multidb

c196a8d

Merge branch 'multidb' of https://github.com/pl04351820/beam into mul…

994f7f6

…tidb

github-actions bot added the Next Action: Reviewers label Aug 14, 2023

Remove databaseRef in WriteFn

1cccde4

pl04351820 force-pushed the multidb branch from 9b1f94e to 1cccde4 Compare August 14, 2023 06:38

retrigger checks

fb16793

johnjcasey requested changes Aug 14, 2023

View reviewed changes

pl04351820 added 2 commits August 14, 2023 08:54

resolve comments

cddc491

resolve comments

a245bfd

pl04351820 requested a review from johnjcasey August 14, 2023 15:56

johnjcasey reviewed Aug 14, 2023

View reviewed changes

...io/google-cloud-platform/src/main/java/org/apache/beam/sdk/io/gcp/datastore/DatastoreV1.java Show resolved Hide resolved

github-actions bot removed the slow-review label Sep 6, 2023

johnjcasey requested changes Sep 13, 2023

View reviewed changes

pl04351820 added 5 commits October 12, 2023 10:00

Merge branch 'apache:master' into multidb

dfa02c5

Remove the PipelineOption reference

299f878

Update comment

34e1c43

Merge branch 'apache:master' into multidb

0dfd9cc

Merge branch 'multidb' of https://github.com/pl04351820/beam into mul…

64eea6d

…tidb

bvolpato reviewed Oct 15, 2023

View reviewed changes

...io/google-cloud-platform/src/main/java/org/apache/beam/sdk/io/gcp/datastore/DatastoreV1.java Outdated Show resolved Hide resolved

bvolpato suggested changes Oct 15, 2023

View reviewed changes

pl04351820 added 3 commits October 16, 2023 08:43

Update ValueProviders usage

0016bcd

Merge branch 'apache:master' into multidb

8fd0fd2

Merge branch 'multidb' of https://github.com/pl04351820/beam into mul…

18b54a1

…tidb

pl04351820 requested a review from johnjcasey October 16, 2023 18:29

Merge branch 'apache:master' into multidb

b5e2b1c

pl04351820 requested a review from bvolpato October 16, 2023 22:14

pl04351820 added 3 commits October 17, 2023 09:11

Merge branch 'apache:master' into multidb

3678e32

Merge branch 'apache:master' into multidb

fd53184

Merge branch 'apache:master' into multidb

2ae28d1

bvolpato approved these changes Oct 30, 2023

View reviewed changes

johnjcasey approved these changes Oct 31, 2023

View reviewed changes

johnjcasey merged commit 181296d into apache:master Oct 31, 2023

damccorm added a commit that referenced this pull request Nov 6, 2023

Revert "Support DatabaseID in Datastore beam connector (#27987)"

76d6aaa

This reverts commit 181296d.

damccorm added a commit that referenced this pull request Nov 6, 2023

Revert "Support DatabaseID in Datastore beam connector (#27987)"

33532a4

This reverts commit 181296d.

damccorm added a commit that referenced this pull request Nov 6, 2023

Revert "Support DatabaseID in Datastore beam connector (#27987)" (#29317

827afb9

) This reverts commit 181296d.

damccorm added a commit that referenced this pull request Nov 6, 2023

Revert "Support DatabaseID in Datastore beam connector (#27987)" (#29318

0fdf404

) This reverts commit 181296d.

pl04351820 mentioned this pull request Dec 19, 2023

Support DatabaseID in Datastore beam connector #29815

Merged

blech75 mentioned this pull request Jul 25, 2024

[Feature Request]: add support for Datastore Database ID in Python SDK #31988

Closed

17 tasks

Conversation

pl04351820 commented Aug 14, 2023 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

github-actions bot commented Aug 14, 2023

Uh oh!

bvolpato commented Aug 14, 2023

Uh oh!

pl04351820 commented Aug 14, 2023

Uh oh!

Uh oh!

Uh oh!

Uh oh!

johnjcasey Aug 14, 2023

Choose a reason for hiding this comment

Uh oh!

pl04351820 Aug 14, 2023

Choose a reason for hiding this comment

Uh oh!

johnjcasey Aug 14, 2023

Choose a reason for hiding this comment

Uh oh!

johnjcasey Aug 14, 2023

Choose a reason for hiding this comment

Uh oh!

pl04351820 Aug 14, 2023

Choose a reason for hiding this comment

Uh oh!

johnjcasey Aug 28, 2023

Choose a reason for hiding this comment

Uh oh!

Uh oh!

johnjcasey left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

bvolpato left a comment

Choose a reason for hiding this comment

Uh oh!

pl04351820 commented Oct 17, 2023

Uh oh!

pl04351820 commented Oct 17, 2023

Uh oh!

bvolpato commented Oct 30, 2023

Uh oh!

bvolpato left a comment

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

pl04351820 commented Aug 14, 2023 •

edited

Loading