New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
[BEAM-1542] Add a possibility to pass a project that hosts Cloud Spanner instance. #8404
Conversation
...o/google-cloud-platform/src/test/java/org/apache/beam/sdk/io/gcp/spanner/SpannerWriteIT.java
Show resolved
Hide resolved
2a04c99
to
122b074
Compare
…e to Cloud Spanner IO ITs.
Run Java PostCommit |
@kennknowles, @chamikaramj could you help with the merge? Thank you. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM. Thanks.
@@ -61,6 +62,12 @@ | |||
|
|||
/** Pipeline options for this test. */ | |||
public interface SpannerTestPipelineOptions extends TestPipelineOptions { | |||
@Description("Project that hosts Spanner instance") | |||
@Nullable | |||
String getInstanceProjectId(); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I am very slightly concerned, because pipeline options are a global namespace and this option does not have "spanner" in the name.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Seems like this is only for integration tests ?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yes. Do pipeline options defined in ITs pollute the global namespace, @kennknowles ?
If so, there are other options in this and other tests that this concern applies to.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
As long as the PipelineOption interface isn't being registered via a ServiceLoader automatically, a call to PipelineOptionsFactory.resetCache() after each test in a @TearDown
method will remove the pollution of the namespace. If it is being registered via serviceloader then people have to be careful around name choices/classpaths.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The PR Adds a possibility to pass a project that hosts Cloud Spanner instance to Cloud Spanner IO ITs. This is useful in test scenarios when Cloud Spanner instance lives in a different project than the one where test pipeline is executed.
If new option is not specified, the test keeps the existing behavior.
Post-Commit Tests Status (on master branch)
Pre-Commit Tests Status (on master branch)
See .test-infra/jenkins/README for trigger phrase, status and link of all Jenkins jobs.