You are viewing a plain text version of this content. The canonical link for it is here.
Posted to github@beam.apache.org by "nuggetwheat (via GitHub)" <gi...@apache.org> on 2023/01/20 22:59:55 UTC
[GitHub] [beam] nuggetwheat opened a new pull request, #25108: Remove databaseRole from metadata DB requests in SpannerIO.ReadChange…
nuggetwheat opened a new pull request, #25108:
URL: https://github.com/apache/beam/pull/25108
This change removes the database role from the SpannerConfig that is used to access the Spanner Change Stream metadata database. The role shouldn't be passed to the Change Stream metadata database because there are no roles defined on that database. Addresses #25105.
--
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: github-unsubscribe@beam.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [beam] johnjcasey commented on pull request #25108: Remove databaseRole from metadata DB requests in SpannerIO.ReadChange…
Posted by "johnjcasey (via GitHub)" <gi...@apache.org>.
johnjcasey commented on PR #25108:
URL: https://github.com/apache/beam/pull/25108#issuecomment-1403886746
yep, we got it merged to master as well
--
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: github-unsubscribe@beam.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [beam] Abacn commented on pull request #25108: Remove databaseRole from metadata DB requests in SpannerIO.ReadChange…
Posted by "Abacn (via GitHub)" <gi...@apache.org>.
Abacn commented on PR #25108:
URL: https://github.com/apache/beam/pull/25108#issuecomment-1399130618
Sorry if I was not clear. master means apache/beam:master branch. I've done that for this PR
--
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: github-unsubscribe@beam.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [beam] johnjcasey commented on pull request #25108: Remove databaseRole from metadata DB requests in SpannerIO.ReadChange…
Posted by "johnjcasey (via GitHub)" <gi...@apache.org>.
johnjcasey commented on PR #25108:
URL: https://github.com/apache/beam/pull/25108#issuecomment-1400478465
Run Java_GCP_IO_Direct PreCommit
--
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: github-unsubscribe@beam.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [beam] nuggetwheat closed pull request #25108: Remove databaseRole from metadata DB requests in SpannerIO.ReadChange…
Posted by "nuggetwheat (via GitHub)" <gi...@apache.org>.
nuggetwheat closed pull request #25108: Remove databaseRole from metadata DB requests in SpannerIO.ReadChange…
URL: https://github.com/apache/beam/pull/25108
--
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: github-unsubscribe@beam.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [beam] johnjcasey commented on pull request #25108: Remove databaseRole from metadata DB requests in SpannerIO.ReadChange…
Posted by "johnjcasey (via GitHub)" <gi...@apache.org>.
johnjcasey commented on PR #25108:
URL: https://github.com/apache/beam/pull/25108#issuecomment-1400730090
It looks like the GCP precommit is failing with a timeout. I can't merge when it has failed after retrying it
--
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: github-unsubscribe@beam.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [beam] Abacn commented on pull request #25108: Remove databaseRole from metadata DB requests in SpannerIO.ReadChange…
Posted by "Abacn (via GitHub)" <gi...@apache.org>.
Abacn commented on PR #25108:
URL: https://github.com/apache/beam/pull/25108#issuecomment-1399145271
I see the same fix #24177 for another use case. The appear to have same cause. Overwrite it to null here just fix this single use case. Preferably we should fix the underlying logic.
--
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: github-unsubscribe@beam.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [beam] Abacn commented on pull request #25108: Remove databaseRole from metadata DB requests in SpannerIO.ReadChange…
Posted by "Abacn (via GitHub)" <gi...@apache.org>.
Abacn commented on PR #25108:
URL: https://github.com/apache/beam/pull/25108#issuecomment-1399057655
Hi, thanks for the quick fix. Could you please first set base branch master, after merged, then cherry pick the merged commit on the master branch into release-2.45.0 branch? This guarantees every commit in release branch is seen on master
--
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: github-unsubscribe@beam.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [beam] Abacn commented on pull request #25108: Remove databaseRole from metadata DB requests in SpannerIO.ReadChange…
Posted by "Abacn (via GitHub)" <gi...@apache.org>.
Abacn commented on PR #25108:
URL: https://github.com/apache/beam/pull/25108#issuecomment-1399143929
Well despite the base branch issue, shouldn't the correct behavior to be, without setting SpannerConfig.setDatabaseRole(null) the pipeline should just work fine? Sounds weird to me that we need to forcefully overwrite this option there.
--
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: github-unsubscribe@beam.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [beam] johnjcasey commented on pull request #25108: Remove databaseRole from metadata DB requests in SpannerIO.ReadChange…
Posted by "johnjcasey (via GitHub)" <gi...@apache.org>.
johnjcasey commented on PR #25108:
URL: https://github.com/apache/beam/pull/25108#issuecomment-1400479263
Agreed with @Abacn . is there a general case fix instead?
--
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: github-unsubscribe@beam.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [beam] johnjcasey merged pull request #25108: Remove databaseRole from metadata DB requests in SpannerIO.ReadChange…
Posted by "johnjcasey (via GitHub)" <gi...@apache.org>.
johnjcasey merged PR #25108:
URL: https://github.com/apache/beam/pull/25108
--
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: github-unsubscribe@beam.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [beam] johnjcasey commented on pull request #25108: Remove databaseRole from metadata DB requests in SpannerIO.ReadChange…
Posted by "johnjcasey (via GitHub)" <gi...@apache.org>.
johnjcasey commented on PR #25108:
URL: https://github.com/apache/beam/pull/25108#issuecomment-1400729017
Lets create an issue, and have it fixed more robustly.
We can merge this workaround into the main branch and this release branch for now
--
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: github-unsubscribe@beam.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [beam] nuggetwheat commented on pull request #25108: Remove databaseRole from metadata DB requests in SpannerIO.ReadChange…
Posted by "nuggetwheat (via GitHub)" <gi...@apache.org>.
nuggetwheat commented on PR #25108:
URL: https://github.com/apache/beam/pull/25108#issuecomment-1399087923
> Hi, thanks for the quick fix. Could you please first set base branch master, after merged, then cherry pick the merged commit on the master branch into release-2.45.0 branch? This guarantees every commit in release branch is seen on master
I added the commit to master, then cherry-picked it back into release-2.45.0. Is that sufficient, or do I need to create a new pull request?
--
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: github-unsubscribe@beam.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [beam] nuggetwheat commented on pull request #25108: Remove databaseRole from metadata DB requests in SpannerIO.ReadChange…
Posted by "nuggetwheat (via GitHub)" <gi...@apache.org>.
nuggetwheat commented on PR #25108:
URL: https://github.com/apache/beam/pull/25108#issuecomment-1400709831
When reading from change streams there are two databases involved, 1) the primary database that contains the change stream, and 2) a metadata database (maintained by Beam) that holds partition progress state so a job can be restarted and pick up where it left off. To create a spanner config to access the metadata database, the following pattern is used:
```
SpannerConfig metadataConfig = spannerConfig.toBuilder()
.setInstanceId(metadataInstanceId)
.setDatabaseId(metadataDatabaseId)
.build();
```
The metadata config (`metadataConfig`) is initialized using the primary database config (`spannerConfig`) since most of the fields are the same and then the ones that are different get overwritten. With the Fine-Grained Access Control project, we've added a new config field _databaseRole_ that should only be set for primary database access. Unfortunately the pattern above blindly copies this field to the metadata config. A better approach might be to write a metadata config create function that copies a whitelisted set of fields from the primary config and leaves all of the other fields empty. I can implement something like this now if you'd like. Otherwise, I can create an issue and assign it to someone on the change stream team who maintains this code. What are your thoughts?
--
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: github-unsubscribe@beam.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [beam] johnjcasey commented on pull request #25108: Remove databaseRole from metadata DB requests in SpannerIO.ReadChange…
Posted by "johnjcasey (via GitHub)" <gi...@apache.org>.
johnjcasey commented on PR #25108:
URL: https://github.com/apache/beam/pull/25108#issuecomment-1400732168
Run Java_GCP_IO_Direct PreCommit
--
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: github-unsubscribe@beam.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [beam] Abacn commented on pull request #25108: Remove databaseRole from metadata DB requests in SpannerIO.ReadChange…
Posted by "Abacn (via GitHub)" <gi...@apache.org>.
Abacn commented on PR #25108:
URL: https://github.com/apache/beam/pull/25108#issuecomment-1402105804
@johnjcasey FYI: https://github.com/apache/beam/pull/25108#issuecomment-1399057655 this change was not in master; release-2.45.0 and master has diverged
--
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: github-unsubscribe@beam.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org