[DO NOT MERGE] [BEAM-7590] Converting JDBC Pipeline Options Map to PipelineOptions. #8928
Conversation
riazela
force-pushed
the
pipeline_option
branch
4 times, most recently
from
3e7a315
to
302efad
Compare
|
Run Java_Examples_Dataflow PreCommit |
5 similar comments
|
Run Java_Examples_Dataflow PreCommit |
|
Run Java_Examples_Dataflow PreCommit |
|
Run Java_Examples_Dataflow PreCommit |
|
Run Java_Examples_Dataflow PreCommit |
|
Run Java_Examples_Dataflow PreCommit |
| @SuppressWarnings("unchecked") | ||
| public static Class<? extends PipelineOptions> getPipelineOptionsInterface( | ||
| PipelineOptions options) { | ||
| if (options.getClass().getInterfaces().length != 1) { |
There was a problem hiding this comment.
PipelineOptions interface validation is fairly strict on making sure that methods have the same default/parameter type/...
You should really support multiple interfaces since its quite common in our codebase.
There was a problem hiding this comment.
Do you mean a pipeline option interface that extends multiple interfaces or the object is implementing multiple interfaces? I couldn't find any example that the pipeline option object is implementing two interfaces. At least if the user uses PipelineOptionsFactory.as() or PipelineOptionsFactory.create() it will return a proxy object that only implements the given interface. (The interface though can be subclass of multiple interfaces itself, and that would be OK.)
There was a problem hiding this comment.
Now that I know your intent, you should instead check to see if the class is an interface using:
https://docs.oracle.com/javase/8/docs/api/java/lang/Class.html#isInterface--
From Class#getInterfaces:
If this object represents an interface, the array contains objects representing all interfaces extended by the interface.
And hence would fail on things like DataflowPipelineOptions which extends multiple interfaces.
There was a problem hiding this comment.
If I call options.getClass() it will not return DataflowPipelineOptions, it will return a proxy class. For instance BigQueryOptions also extends multiple interfaces (Similar to DataflowPipelineOptions); however, if I run the following code:
BigQueryOptions options = PipelineOptionsFactory.as(BigQueryOptions.class);
System.out.println(options.getClass());
System.out.println(options.getClass().isInterface());
System.out.println(PipelineOptionsReflectionSetter.getPipelineOptionsInterface(options));
There will be no exception and the output will be:
class com.sun.proxy.$Proxy25
false
interface org.apache.beam.sdk.io.gcp.bigquery.BigQueryOptions
The reason that I need this method is when the user tries to use reset command, I should set it to its default value. In order to do that, first I try to see what type of pipelineOptions this object is implementing and then using PipelineOptionsFactory I construct an instance of that options and get its default value.
Currently with our use cases and the expected behavior of PipelineOptionsFactory, this should not fail in any use case. Because the method .as() in PipelineOptions and PipelineOptionsFactory returns a proxy object that implements one interface. I think currently, if the user needs its own piplineOptions they need to declare an interface extending PipelineOptions and then use PipelineOptionsFactory to create an instance of it.
|
run java postcommit |
There were two representation of pipeline options in JDBC Driver. This will combine them and we will keep only PipelineOptions Class.
This will slightly change the JDBC set command. Previously if we had set an incorrect option it would ignore it until the execution of an actual query; whereas now, it will throw exception if you try to set an incorrect pipeline option.
Thank you for your contribution! Follow this checklist to help us incorporate your contribution quickly and easily:
R: @username).[BEAM-XXX] Fixes bug in ApproximateQuantiles, where you replaceBEAM-XXXwith the appropriate JIRA issue, if applicable. This will automatically link the pull request to the issue.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.