Thank you for the clarification. I closed the current pull request, I will
create a new Jira Issue for the proposed methods.
Best,
Alireza
On Mon, Jul 8, 2019 at 4:38 PM Lukasz Cwik wrote:
> Ok, since you want parsing to be exactly the same then it does make sense
> to expose some part of the P
Ok, since you want parsing to be exactly the same then it does make sense
to expose some part of the PipelineOptionsFactory to do the parsing.
On the API point of view, it would make sense to break up the reflection
portion to be based upon the object types inside of PipelineOptions
interface (bac
We have definitely tried to rework this a few times in the past. The
biggest problems is that some changes to pipeline options require multiple
values to change at once. For example, changing the runner might require
some options to be set and others reset before the options are valid.
I'll try to
Before this PR, the set command was using a map to store values and then it
was using PipelineOptionsFactory#fromArgs to parse those values. Therefore,
by using PipelieOptionsFactory#parseObjects, we keep the same value parsing
behavior for the SET command as before. Using PipelineOptionsFactory fo
I see, in the current PR it seems like we are trying to adopt the parsing
logic of PipelineOptions command line value parsing to all SQL usecases
since we are exposing the parseOption method to be used in the
PipelineOptionsReflectionSetter#setOption.
I should have asked in my earlier e-mail wheth
The proposed API assumes you already have a property name and a value
parsed somehow, and now want to update a field on a pre-existing options
object with that value, so there is no assumption about parsing being the
same or not. E.g. if you set a property called `runner` to a string value
`DirectR
Do we want SQL argument parsing to always be 1-1 with how command line
parsing is being done?
Note that this is different from the JSON <-> PipelineOptions conversion.
I can see why the wrapper makes sense, just want to make sure that the JDBC
SET command aligns with what we are trying to expose.
I think we thought about this approach but decided to get rid of the map
representation wherever we can while still supporting setting of the
options by name.
One of the lesser important downsides of keeping the map around is that we
will need to do `fromArgs` at least twice.
Another downside is
Not sure, based upon the JIRA description it seems like you want early
validation of PipelineOptions. Couldn't you maintain the map of pipeline
options and every time one is added call PipelineOptionsFactory.fromArgs
discarding the result just for the error checking?
On Tue, Jun 25, 2019 at 10:12
Not sure. One solution might be moving the PipelineOptionsReflectionSetter
class to SQL package and make it package private. This will prevent the
exposure but the downside would be I need to make
PipelineOptionsFactory.parseObjects() public or duplicate its code. Do you
think this approach might b
That makes sense. I took a look at your PR, is there a way to do it without
exposing the reflection capabilities to pipeline authors?
On Mon, Jun 24, 2019 at 2:20 PM Alireza Samadian
wrote:
> Hi all,
>
> I am writing to ask if it is OK to slightly change the behaviour of SET
> command in JDBC co
Hi all,
I am writing to ask if it is OK to slightly change the behaviour of SET
command in JDBC connection of Beam SQL. Currently, if we try to use set
command for an option that does not exist or setting an option to an
illegal value, it does not show any error until we run a query. This means
on
12 matches
Mail list logo
