You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@beam.apache.org by Romain Manni-Bucau <rm...@gmail.com> on 2018/02/13 14:42:00 UTC
PipelineOptions fromSystemProps?
Hi guys,
there are hacks in beam testing code to read the args from a system
property but I wonder if we shouldnt add a
PipelineOptionsFactory.fromSystemProperties().
It would iterate over the system properties and take all --xxx=foo as
potential argument it tries to bind.
Rational behind that is to enable users to wrap the pipeline API but still
expose the pipeline options to end users for advanced cases.
Any discussion on this kind of usages already? What do you think of this
proposal?
Side note: we can think about a fromEnv() too.
Romain Manni-Bucau
@rmannibucau <https://twitter.com/rmannibucau> | Blog
<https://rmannibucau.metawerx.net/> | Old Blog
<http://rmannibucau.wordpress.com> | Github <https://github.com/rmannibucau> |
LinkedIn <https://www.linkedin.com/in/rmannibucau> | Book
<https://www.packtpub.com/application-development/java-ee-8-high-performance>
Re: PipelineOptions fromSystemProps?
Posted by Romain Manni-Bucau <rm...@gmail.com>.
will create it now, thanks
Romain Manni-Bucau
@rmannibucau <https://twitter.com/rmannibucau> | Blog
<https://rmannibucau.metawerx.net/> | Old Blog
<http://rmannibucau.wordpress.com> | Github <https://github.com/rmannibucau> |
LinkedIn <https://www.linkedin.com/in/rmannibucau> | Book
<https://www.packtpub.com/application-development/java-ee-8-high-performance>
2018-02-16 17:23 GMT+01:00 Kenneth Knowles <kl...@google.com>:
> Good idea to split the discussion. We can complete PipelineOptions.fromSystemProperties
> immediately for Java and separate the cross-language "multi-env var
> representation" for pipeline options. Probably Python & Go fans have muted
> this thread, after all.
>
> On Fri, Feb 16, 2018 at 8:07 AM, Romain Manni-Bucau <rmannibucau@gmail.com
> > wrote:
>
>> Oh, so the point was than env would be under the portability umbrella
>> versus system properties are not? Kind of makes sense phrased this way for
>> me.
>>
>> Do we want another thread for that?
>>
>>
>> Romain Manni-Bucau
>> @rmannibucau <https://twitter.com/rmannibucau> | Blog
>> <https://rmannibucau.metawerx.net/> | Old Blog
>> <http://rmannibucau.wordpress.com> | Github
>> <https://github.com/rmannibucau> | LinkedIn
>> <https://www.linkedin.com/in/rmannibucau> | Book
>> <https://www.packtpub.com/application-development/java-ee-8-high-performance>
>>
>> 2018-02-16 16:37 GMT+01:00 Kenneth Knowles <kl...@google.com>:
>>
>>>
>>> On Thu, Feb 15, 2018, 11:47 PM Romain Manni-Bucau <rm...@gmail.com>
>>>> wrote:
>>>>
>>>>>
>>>>> 2018-02-15 20:00 GMT+01:00 Kenneth Knowles <kl...@google.com>:
>>>>>
>>>>>> On Thu, Feb 15, 2018 at 12:03 AM, Romain Manni-Bucau <
>>>>>> rmannibucau@gmail.com> wrote:
>>>>>>>
>>>>>>> 2. default properties = env + system properties: this is what all
>>>>>>> config libs do (spring config, tamaya, deltaspike, microprofile-config,
>>>>>>> ...) so it is very comfortable and sane for end users. It enables ops (env)
>>>>>>> and dev (system properties) to feel comfortable for free with this and
>>>>>>> avoids us to have N entry points which has the nice side effect to
>>>>>>> consolidate the overall consistency of the framework/lib.
>>>>>>>
>>>>>>
>>>>>> Can you link to the docs for these examples? I'm sure we'd like to
>>>>>> learn from them.
>>>>>>
>>>>>
>>>>> I'm not the best to find docs but the relevant bits are:
>>>>>
>>>>> https://github.com/apache/deltaspike/blob/842c84bd1767905a19
>>>>> e2af01bdfe88d416d1b2da/deltaspike/core/impl/src/main/java/or
>>>>> g/apache/deltaspike/core/impl/config/DefaultConfigSourceProv
>>>>> ider.java#L57
>>>>> https://github.com/apache/geronimo-config/blob/trunk/impl/sr
>>>>> c/main/java/org/apache/geronimo/config/DefaultConfigBuilder.java#L182
>>>>> https://github.com/spring-projects/spring-framework/blob/a0b
>>>>> ce618c2f51d8af1fc00ee2c3868ba2c8e0045/spring-core/src/main/j
>>>>> ava/org/springframework/core/env/StandardEnvironment.java#L78
>>>>>
>>>>> (and there are others a bit less mainstream)
>>>>>
>>>>> The point is just that it is very common to read both and would be
>>>>> weird to not have that feature if we read from system props.
>>>>>
>>>>
>>> I'm convinced that it is common. It isn't clear that it is useful. But
>>> since it has very little impact I'm fine with it. We can take the Java
>>> naming problem to the PR.
>>>
>>> Now the schema for this collection of env vars is a cross-language
>>> question, so we need to make sure Python and Go are on board.
>>>
>>> Kenn
>>>
>>
>>
>
Re: PipelineOptions fromSystemProps?
Posted by Kenneth Knowles <kl...@google.com>.
Good idea to split the discussion. We can complete
PipelineOptions.fromSystemProperties immediately for Java and separate the
cross-language "multi-env var representation" for pipeline options.
Probably Python & Go fans have muted this thread, after all.
On Fri, Feb 16, 2018 at 8:07 AM, Romain Manni-Bucau <rm...@gmail.com>
wrote:
> Oh, so the point was than env would be under the portability umbrella
> versus system properties are not? Kind of makes sense phrased this way for
> me.
>
> Do we want another thread for that?
>
>
> Romain Manni-Bucau
> @rmannibucau <https://twitter.com/rmannibucau> | Blog
> <https://rmannibucau.metawerx.net/> | Old Blog
> <http://rmannibucau.wordpress.com> | Github
> <https://github.com/rmannibucau> | LinkedIn
> <https://www.linkedin.com/in/rmannibucau> | Book
> <https://www.packtpub.com/application-development/java-ee-8-high-performance>
>
> 2018-02-16 16:37 GMT+01:00 Kenneth Knowles <kl...@google.com>:
>
>>
>> On Thu, Feb 15, 2018, 11:47 PM Romain Manni-Bucau <rm...@gmail.com>
>>> wrote:
>>>
>>>>
>>>> 2018-02-15 20:00 GMT+01:00 Kenneth Knowles <kl...@google.com>:
>>>>
>>>>> On Thu, Feb 15, 2018 at 12:03 AM, Romain Manni-Bucau <
>>>>> rmannibucau@gmail.com> wrote:
>>>>>>
>>>>>> 2. default properties = env + system properties: this is what all
>>>>>> config libs do (spring config, tamaya, deltaspike, microprofile-config,
>>>>>> ...) so it is very comfortable and sane for end users. It enables ops (env)
>>>>>> and dev (system properties) to feel comfortable for free with this and
>>>>>> avoids us to have N entry points which has the nice side effect to
>>>>>> consolidate the overall consistency of the framework/lib.
>>>>>>
>>>>>
>>>>> Can you link to the docs for these examples? I'm sure we'd like to
>>>>> learn from them.
>>>>>
>>>>
>>>> I'm not the best to find docs but the relevant bits are:
>>>>
>>>> https://github.com/apache/deltaspike/blob/842c84bd1767905a19
>>>> e2af01bdfe88d416d1b2da/deltaspike/core/impl/src/main/java/
>>>> org/apache/deltaspike/core/impl/config/DefaultConfigSourc
>>>> eProvider.java#L57
>>>> https://github.com/apache/geronimo-config/blob/trunk/impl/sr
>>>> c/main/java/org/apache/geronimo/config/DefaultConfigBuilder.java#L182
>>>> https://github.com/spring-projects/spring-framework/blob/a0b
>>>> ce618c2f51d8af1fc00ee2c3868ba2c8e0045/spring-core/src/main/j
>>>> ava/org/springframework/core/env/StandardEnvironment.java#L78
>>>>
>>>> (and there are others a bit less mainstream)
>>>>
>>>> The point is just that it is very common to read both and would be
>>>> weird to not have that feature if we read from system props.
>>>>
>>>
>> I'm convinced that it is common. It isn't clear that it is useful. But
>> since it has very little impact I'm fine with it. We can take the Java
>> naming problem to the PR.
>>
>> Now the schema for this collection of env vars is a cross-language
>> question, so we need to make sure Python and Go are on board.
>>
>> Kenn
>>
>
>
Re: PipelineOptions fromSystemProps?
Posted by Romain Manni-Bucau <rm...@gmail.com>.
Oh, so the point was than env would be under the portability umbrella
versus system properties are not? Kind of makes sense phrased this way for
me.
Do we want another thread for that?
Romain Manni-Bucau
@rmannibucau <https://twitter.com/rmannibucau> | Blog
<https://rmannibucau.metawerx.net/> | Old Blog
<http://rmannibucau.wordpress.com> | Github <https://github.com/rmannibucau> |
LinkedIn <https://www.linkedin.com/in/rmannibucau> | Book
<https://www.packtpub.com/application-development/java-ee-8-high-performance>
2018-02-16 16:37 GMT+01:00 Kenneth Knowles <kl...@google.com>:
>
> On Thu, Feb 15, 2018, 11:47 PM Romain Manni-Bucau <rm...@gmail.com>
>> wrote:
>>
>>>
>>> 2018-02-15 20:00 GMT+01:00 Kenneth Knowles <kl...@google.com>:
>>>
>>>> On Thu, Feb 15, 2018 at 12:03 AM, Romain Manni-Bucau <
>>>> rmannibucau@gmail.com> wrote:
>>>>>
>>>>> 2. default properties = env + system properties: this is what all
>>>>> config libs do (spring config, tamaya, deltaspike, microprofile-config,
>>>>> ...) so it is very comfortable and sane for end users. It enables ops (env)
>>>>> and dev (system properties) to feel comfortable for free with this and
>>>>> avoids us to have N entry points which has the nice side effect to
>>>>> consolidate the overall consistency of the framework/lib.
>>>>>
>>>>
>>>> Can you link to the docs for these examples? I'm sure we'd like to
>>>> learn from them.
>>>>
>>>
>>> I'm not the best to find docs but the relevant bits are:
>>>
>>> https://github.com/apache/deltaspike/blob/842c84bd1767905a19
>>> e2af01bdfe88d416d1b2da/deltaspike/core/impl/src/main/
>>> java/org/apache/deltaspike/core/impl/config/DefaultConfig
>>> SourceProvider.java#L57
>>> https://github.com/apache/geronimo-config/blob/trunk/impl/
>>> src/main/java/org/apache/geronimo/config/DefaultConfigBuilder.java#L182
>>> https://github.com/spring-projects/spring-framework/blob/a0b
>>> ce618c2f51d8af1fc00ee2c3868ba2c8e0045/spring-core/src/main/
>>> java/org/springframework/core/env/StandardEnvironment.java#L78
>>>
>>> (and there are others a bit less mainstream)
>>>
>>> The point is just that it is very common to read both and would be weird
>>> to not have that feature if we read from system props.
>>>
>>
> I'm convinced that it is common. It isn't clear that it is useful. But
> since it has very little impact I'm fine with it. We can take the Java
> naming problem to the PR.
>
> Now the schema for this collection of env vars is a cross-language
> question, so we need to make sure Python and Go are on board.
>
> Kenn
>
Re: PipelineOptions fromSystemProps?
Posted by Kenneth Knowles <kl...@google.com>.
> On Thu, Feb 15, 2018, 11:47 PM Romain Manni-Bucau <rm...@gmail.com>
> wrote:
>
>>
>> 2018-02-15 20:00 GMT+01:00 Kenneth Knowles <kl...@google.com>:
>>
>>> On Thu, Feb 15, 2018 at 12:03 AM, Romain Manni-Bucau <
>>> rmannibucau@gmail.com> wrote:
>>>>
>>>> 2. default properties = env + system properties: this is what all
>>>> config libs do (spring config, tamaya, deltaspike, microprofile-config,
>>>> ...) so it is very comfortable and sane for end users. It enables ops (env)
>>>> and dev (system properties) to feel comfortable for free with this and
>>>> avoids us to have N entry points which has the nice side effect to
>>>> consolidate the overall consistency of the framework/lib.
>>>>
>>>
>>> Can you link to the docs for these examples? I'm sure we'd like to learn
>>> from them.
>>>
>>
>> I'm not the best to find docs but the relevant bits are:
>>
>> https://github.com/apache/deltaspike/blob/842c84bd1767905a19e2af01bdfe88
>> d416d1b2da/deltaspike/core/impl/src/main/java/org/apache/
>> deltaspike/core/impl/config/DefaultConfigSourceProvider.java#L57
>> https://github.com/apache/geronimo-config/blob/trunk/
>> impl/src/main/java/org/apache/geronimo/config/
>> DefaultConfigBuilder.java#L182
>> https://github.com/spring-projects/spring-framework/blob/
>> a0bce618c2f51d8af1fc00ee2c3868ba2c8e0045/spring-core/src/
>> main/java/org/springframework/core/env/StandardEnvironment.java#L78
>>
>> (and there are others a bit less mainstream)
>>
>> The point is just that it is very common to read both and would be weird
>> to not have that feature if we read from system props.
>>
>
I'm convinced that it is common. It isn't clear that it is useful. But
since it has very little impact I'm fine with it. We can take the Java
naming problem to the PR.
Now the schema for this collection of env vars is a cross-language
question, so we need to make sure Python and Go are on board.
Kenn
Re: PipelineOptions fromSystemProps?
Posted by Reuven Lax <re...@google.com>.
I don't have a strong feeling about null here (though we're Java 8 now -
can't we start using Optional instead?). However it's always easier to add
things and harder to remove things. If it's not really needed now, id vote
to leave it out of this PR and add later if necessary.
On Thu, Feb 15, 2018, 11:47 PM Romain Manni-Bucau <rm...@gmail.com>
wrote:
>
> 2018-02-15 20:00 GMT+01:00 Kenneth Knowles <kl...@google.com>:
>
>> On Thu, Feb 15, 2018 at 12:03 AM, Romain Manni-Bucau <
>> rmannibucau@gmail.com> wrote:
>>>
>>> 2. default properties = env + system properties: this is what all config
>>> libs do (spring config, tamaya, deltaspike, microprofile-config, ...) so it
>>> is very comfortable and sane for end users. It enables ops (env) and dev
>>> (system properties) to feel comfortable for free with this and avoids us to
>>> have N entry points which has the nice side effect to consolidate the
>>> overall consistency of the framework/lib.
>>>
>>
>> Can you link to the docs for these examples? I'm sure we'd like to learn
>> from them.
>>
>
> I'm not the best to find docs but the relevant bits are:
>
>
> https://github.com/apache/deltaspike/blob/842c84bd1767905a19e2af01bdfe88d416d1b2da/deltaspike/core/impl/src/main/java/org/apache/deltaspike/core/impl/config/DefaultConfigSourceProvider.java#L57
>
> https://github.com/apache/geronimo-config/blob/trunk/impl/src/main/java/org/apache/geronimo/config/DefaultConfigBuilder.java#L182
>
> https://github.com/spring-projects/spring-framework/blob/a0bce618c2f51d8af1fc00ee2c3868ba2c8e0045/spring-core/src/main/java/org/springframework/core/env/StandardEnvironment.java#L78
>
> (and there are others a bit less mainstream)
>
> The point is just that it is very common to read both and would be weird
> to not have that feature if we read from system props.
>
>
>>
>>
>>> 4. support null: I'd like to support null as a valid valid for
>>> properties. The rational here is to make it neat for end user and support:
>>> fromProperties(loadProps()) with a loadProps in the user codebase which can
>>> return null. We just default to create(). For us it iss pretty much nothing
>>> but it enables end users to not care about it. Concretely one null check
>>> versus millions (or thousands at least ;)).
>>>
>>
>>
>> https://www.infoq.com/presentations/Null-References-The-Billion-Dollar-Mistake-Tony-Hoare
>>
>
> :), concretely this applies only to your own codebase, as soon as you
> start to expose an external API (intended pleonasm ;)) then you are forced
> to handle null cause you can't help users to pass null. It doesn't imply
> for all API in beam, for instance @ProcessElement can assume it is not null
> in several cases but this fromXXX() is 100% delegated to the users. Here
> the compromise is the cost of handling it once for all vs the cost to ask
> all users to do it. My opinion is that it doesnt cost anything for beam, it
> is safe for beam and therefore it is way better to put it in beam than ask
> all users to care about it.
>
> That said I don't want to spend my week spare time speaking of that since
> the feature is straight forward and not impacting the runtime. This is why
> i droppped it from this PR.
>
>
>>
>> Kenn
>>
>
>
Re: PipelineOptions fromSystemProps?
Posted by Romain Manni-Bucau <rm...@gmail.com>.
2018-02-15 20:00 GMT+01:00 Kenneth Knowles <kl...@google.com>:
> On Thu, Feb 15, 2018 at 12:03 AM, Romain Manni-Bucau <
> rmannibucau@gmail.com> wrote:
>>
>> 2. default properties = env + system properties: this is what all config
>> libs do (spring config, tamaya, deltaspike, microprofile-config, ...) so it
>> is very comfortable and sane for end users. It enables ops (env) and dev
>> (system properties) to feel comfortable for free with this and avoids us to
>> have N entry points which has the nice side effect to consolidate the
>> overall consistency of the framework/lib.
>>
>
> Can you link to the docs for these examples? I'm sure we'd like to learn
> from them.
>
I'm not the best to find docs but the relevant bits are:
https://github.com/apache/deltaspike/blob/842c84bd1767905a19e2af01bdfe88d416d1b2da/deltaspike/core/impl/src/main/java/org/apache/deltaspike/core/impl/config/DefaultConfigSourceProvider.java#L57
https://github.com/apache/geronimo-config/blob/trunk/impl/src/main/java/org/apache/geronimo/config/DefaultConfigBuilder.java#L182
https://github.com/spring-projects/spring-framework/blob/a0bce618c2f51d8af1fc00ee2c3868ba2c8e0045/spring-core/src/main/java/org/springframework/core/env/StandardEnvironment.java#L78
(and there are others a bit less mainstream)
The point is just that it is very common to read both and would be weird to
not have that feature if we read from system props.
>
>
>> 4. support null: I'd like to support null as a valid valid for
>> properties. The rational here is to make it neat for end user and support:
>> fromProperties(loadProps()) with a loadProps in the user codebase which can
>> return null. We just default to create(). For us it iss pretty much nothing
>> but it enables end users to not care about it. Concretely one null check
>> versus millions (or thousands at least ;)).
>>
>
> https://www.infoq.com/presentations/Null-References-
> The-Billion-Dollar-Mistake-Tony-Hoare
>
:), concretely this applies only to your own codebase, as soon as you start
to expose an external API (intended pleonasm ;)) then you are forced to
handle null cause you can't help users to pass null. It doesn't imply for
all API in beam, for instance @ProcessElement can assume it is not null in
several cases but this fromXXX() is 100% delegated to the users. Here the
compromise is the cost of handling it once for all vs the cost to ask all
users to do it. My opinion is that it doesnt cost anything for beam, it is
safe for beam and therefore it is way better to put it in beam than ask all
users to care about it.
That said I don't want to spend my week spare time speaking of that since
the feature is straight forward and not impacting the runtime. This is why
i droppped it from this PR.
>
> Kenn
>
Re: PipelineOptions fromSystemProps?
Posted by Kenneth Knowles <kl...@google.com>.
On Thu, Feb 15, 2018 at 12:03 AM, Romain Manni-Bucau <rm...@gmail.com>
wrote:
>
> 2. default properties = env + system properties: this is what all config
> libs do (spring config, tamaya, deltaspike, microprofile-config, ...) so it
> is very comfortable and sane for end users. It enables ops (env) and dev
> (system properties) to feel comfortable for free with this and avoids us to
> have N entry points which has the nice side effect to consolidate the
> overall consistency of the framework/lib.
>
Can you link to the docs for these examples? I'm sure we'd like to learn
from them.
> 4. support null: I'd like to support null as a valid valid for properties.
> The rational here is to make it neat for end user and support:
> fromProperties(loadProps()) with a loadProps in the user codebase which can
> return null. We just default to create(). For us it iss pretty much nothing
> but it enables end users to not care about it. Concretely one null check
> versus millions (or thousands at least ;)).
>
https://www.infoq.com/presentations/Null-References-The-Billion-Dollar-Mistake-Tony-Hoare
Kenn
Re: PipelineOptions fromSystemProps?
Posted by Romain Manni-Bucau <rm...@gmail.com>.
After Kenneth review we have a few points to discuss and I'd like to
benefit from the mail to add some more thoughts than on github:
1. pivot format or not: in my PR I used the CLI format to just delegate to
fromArgs the parsing once the properties were converted to args. I like
that cause it enforces args syntax to work and prevent to abuse of an
internal pivot format which would support more than what a CLI can support
- like the JSON syntax which can be abused today. It also makes the code
very simple with a single entry code path which is quite neat IMHO.
2. default properties = env + system properties: this is what all config
libs do (spring config, tamaya, deltaspike, microprofile-config, ...) so it
is very comfortable and sane for end users. It enables ops (env) and dev
(system properties) to feel comfortable for free with this and avoids us to
have N entry points which has the nice side effect to consolidate the
overall consistency of the framework/lib.
3. default mecanism: I have in the PR a fromJvm() which uses the previous
(2) mecanism and filters entries using "beam." prefix. Intent is to
encourage integrators to use that instead of a custom prefix by default to
have some consistency accross frameworks. I understand there is not always
a single set of config per JVM but it is also a very common case to have
it. I estimate it at 85% for the pipeline creation part (I'm not speaking
of the workers here since it is way before that). Personally I think if the
investment is low (3 lines) and we cover some more use cases it does worth
it anyway. In this case it is a lot of cases so really important to do the
small quick win associated.
4. support null: I'd like to support null as a valid valid for properties.
The rational here is to make it neat for end user and support:
fromProperties(loadProps()) with a loadProps in the user codebase which can
return null. We just default to create(). For us it iss pretty much nothing
but it enables end users to not care about it. Concretely one null check
versus millions (or thousands at least ;)).
To summarize the overall philosophy: code is very simple and I don't care
much what it looks like, what I worry about is if the solution is usable or
not and if it is easy *for the end users* whatever it costs at beam level
because this part is not very impacting for beam itself. I understand the
"beam doesnt use a null parameter" spirit but it impacts users so it
shouldn't be a surfacing constraint and we should just handle it for users
IMHO. Same for env/system properties discussion. If we handle system
properties ops will expect we handle env variables so just do it, doesn't
cost anything for us and enable more use cases so it is a clear win-win.
Hope it makes sense
Romain Manni-Bucau
@rmannibucau <https://twitter.com/rmannibucau> | Blog
<https://rmannibucau.metawerx.net/> | Old Blog
<http://rmannibucau.wordpress.com> | Github <https://github.com/rmannibucau> |
LinkedIn <https://www.linkedin.com/in/rmannibucau> | Book
<https://www.packtpub.com/application-development/java-ee-8-high-performance>
2018-02-14 15:48 GMT+01:00 Romain Manni-Bucau <rm...@gmail.com>:
> this is part of the PR without any transformation - as requested. I'm
> happy to do a follow up PR to do the same transformations than in
> deltaspike for instance: https://github.com/apache/deltaspike/blob/master/
> deltaspike/core/impl/src/main/java/org/apache/deltaspike/core/impl/config/
> EnvironmentPropertyConfigSource.java#L55 or a more fancy - but real world
> - one like key -> key.substring(prefix.length())
> .toUpperCase(ROOT).replace('.', '_')
>
>
> Romain Manni-Bucau
> @rmannibucau <https://twitter.com/rmannibucau> | Blog
> <https://rmannibucau.metawerx.net/> | Old Blog
> <http://rmannibucau.wordpress.com> | Github
> <https://github.com/rmannibucau> | LinkedIn
> <https://www.linkedin.com/in/rmannibucau> | Book
> <https://www.packtpub.com/application-development/java-ee-8-high-performance>
>
> 2018-02-14 15:42 GMT+01:00 Ismaël Mejía <ie...@gmail.com>:
>
>> +1 to Romain idea with Kenn's approach, we should probably reserve the
>> 'beam.' namespace too because we are already using it for system
>> properties in the spark runner, following along the convention of
>> 'spark.*' in Apache Spark.
>>
>> Any ideas on how to handle this for the env vraiables case ? maybe this?
>> export my.random.namespace="SomeOption=bizzle SomeOtherOption=whatever"
>>
>>
>> On Wed, Feb 14, 2018 at 9:35 AM, Romain Manni-Bucau
>> <rm...@gmail.com> wrote:
>> > FYI created https://github.com/apache/beam/pull/4683 about it
>> >
>> >
>> > Romain Manni-Bucau
>> > @rmannibucau | Blog | Old Blog | Github | LinkedIn | Book
>> >
>> > 2018-02-13 19:21 GMT+01:00 Romain Manni-Bucau <rm...@gmail.com>:
>> >>
>> >> Oki, will try a PR after the classloader one then. Thanks a lot.
>> >>
>> >> Le 13 févr. 2018 19:14, "Lukasz Cwik" <lc...@google.com> a écrit :
>> >>>
>> >>> The one in Dataflow 1.x was one system property that contained all the
>> >>> JSON so it wasn't exactly what you were looking for.
>> >>>
>> >>> On Tue, Feb 13, 2018 at 9:51 AM, Romain Manni-Bucau
>> >>> <rm...@gmail.com> wrote:
>> >>>>
>> >>>> I like your proposal Kenneth. Perfectly fits my use case and
>> deployment
>> >>>> one as well - when ops configure the env without modifying the code.
>> >>>>
>> >>>> How do we move forward on that? Should I send a PR or do you want to
>> >>>> import what was in dataflow?
>> >>>>
>> >>>>
>> >>>> Romain Manni-Bucau
>> >>>> @rmannibucau | Blog | Old Blog | Github | LinkedIn | Book
>> >>>>
>> >>>> 2018-02-13 18:49 GMT+01:00 Lukasz Cwik <lc...@google.com>:
>> >>>>>
>> >>>>> PipelineOptionsFactory.fromSystemProperties did exist in Dataflow
>> 1.x
>> >>>>> and was dropped for the reason that Ken mentioned.
>> >>>>>
>> >>>>> On Tue, Feb 13, 2018 at 9:46 AM, Kenneth Knowles <kl...@google.com>
>> >>>>> wrote:
>> >>>>>>
>> >>>>>> Pipeline options are not global - they are a property of a single
>> job.
>> >>>>>> The TestPipeline reads them from a very particular system property
>> because
>> >>>>>> it is a special testing rule.
>> >>>>>>
>> >>>>>> If you want a generic way to build pipeline options from a set of
>> >>>>>> system properties, it should be from an explicit prefix, not global
>> >>>>>> defaults. So if a user wants to put a bunch of options into
>> properties, they
>> >>>>>> choose a namespace like "my.random.namespace" and everything under
>> it can be
>> >>>>>> interpreted as a pipelineoption:
>> >>>>>>
>> >>>>>> my.random.namespace.SomeOption=bizzle
>> >>>>>> my.random.namespace.SomeOtherOption=whatever
>> >>>>>>
>> >>>>>> And you could do
>> >>>>>> PipelineOptionsFactory.fromSystemProperties("my.random.namespace")
>> >>>>>>
>> >>>>>> We should use the full name of the option not split it with dots -
>> >>>>>> dots represent hierarchy not words separation.
>> >>>>>>
>> >>>>>> interface FooOptions extends PipelineOptions {
>> >>>>>> getSomeOption()
>> >>>>>> getSomeOtherOption()
>> >>>>>> }
>> >>>>>>
>> >>>>>> I think I can be +0.5 on this. We might also reserve a namespace
>> like
>> >>>>>> "beam.TestPipeline.options" and use it for specification of test
>> config.
>> >>>>>> Could be easier than embedding JSON in a property, in some cases.
>> Easier to
>> >>>>>> override little pieces for sure.
>> >>>>>>
>> >>>>>> Kenn
>> >>>>>>
>> >>>>>> On Tue, Feb 13, 2018 at 9:29 AM, Romain Manni-Bucau
>> >>>>>> <rm...@gmail.com> wrote:
>> >>>>>>>
>> >>>>>>> makes sense, do we want beam.foo.bar -> --foo-bar conversion too?
>> >>>>>>>
>> >>>>>>>
>> >>>>>>> Romain Manni-Bucau
>> >>>>>>> @rmannibucau | Blog | Old Blog | Github | LinkedIn | Book
>> >>>>>>>
>> >>>>>>> 2018-02-13 18:19 GMT+01:00 Eugene Kirpichov <kirpichov@google.com
>> >:
>> >>>>>>>>
>> >>>>>>>> Neutral about this one: haven't seen a case where this was
>> needed,
>> >>>>>>>> but don't see anything wrong with it either. One thing I'd
>> recommend if you
>> >>>>>>>> go through with it, extract from system properties under "beam."
>> rather than
>> >>>>>>>> all of them, to avoid clashes.
>> >>>>>>>>
>> >>>>>>>>
>> >>>>>>>> On Tue, Feb 13, 2018, 7:53 AM Jean-Baptiste Onofré <
>> jb@nanthrax.net>
>> >>>>>>>> wrote:
>> >>>>>>>>>
>> >>>>>>>>> Hi Romain,
>> >>>>>>>>>
>> >>>>>>>>> it sounds interesting to me, and doesn't break anything, so +1
>> from
>> >>>>>>>>> my side.
>> >>>>>>>>>
>> >>>>>>>>> Regards
>> >>>>>>>>> JB
>> >>>>>>>>>
>> >>>>>>>>> On 02/13/2018 03:42 PM, Romain Manni-Bucau wrote:
>> >>>>>>>>> > Hi guys,
>> >>>>>>>>> >
>> >>>>>>>>> > there are hacks in beam testing code to read the args from a
>> >>>>>>>>> > system property but
>> >>>>>>>>> > I wonder if we shouldnt add a
>> >>>>>>>>> > PipelineOptionsFactory.fromSystemProperties().
>> >>>>>>>>> >
>> >>>>>>>>> > It would iterate over the system properties and take all
>> >>>>>>>>> > --xxx=foo as potential
>> >>>>>>>>> > argument it tries to bind.
>> >>>>>>>>> >
>> >>>>>>>>> > Rational behind that is to enable users to wrap the pipeline
>> API
>> >>>>>>>>> > but still
>> >>>>>>>>> > expose the pipeline options to end users for advanced cases.
>> >>>>>>>>> >
>> >>>>>>>>> > Any discussion on this kind of usages already? What do you
>> think
>> >>>>>>>>> > of this proposal?
>> >>>>>>>>> >
>> >>>>>>>>> > Side note: we can think about a fromEnv() too.
>> >>>>>>>>> >
>> >>>>>>>>> > Romain Manni-Bucau
>> >>>>>>>>> > @rmannibucau <https://twitter.com/rmannibucau> | Blog
>> >>>>>>>>> > <https://rmannibucau.metawerx.net/> | Old Blog
>> >>>>>>>>> > <http://rmannibucau.wordpress.com> | Github
>> >>>>>>>>> > <https://github.com/rmannibucau> |
>> >>>>>>>>> > LinkedIn <https://www.linkedin.com/in/rmannibucau> | Book
>> >>>>>>>>> >
>> >>>>>>>>> > <https://www.packtpub.com/application-development/java-ee-8-
>> high-performance>
>> >>>>>>>>>
>> >>>>>>>>> --
>> >>>>>>>>> Jean-Baptiste Onofré
>> >>>>>>>>> jbonofre@apache.org
>> >>>>>>>>> http://blog.nanthrax.net
>> >>>>>>>>> Talend - http://www.talend.com
>> >>>>>>>
>> >>>>>>>
>> >>>>>>
>> >>>>>
>> >>>>
>> >>>
>> >
>>
>
>
Re: PipelineOptions fromSystemProps?
Posted by Romain Manni-Bucau <rm...@gmail.com>.
this is part of the PR without any transformation - as requested. I'm happy
to do a follow up PR to do the same transformations than in deltaspike for
instance:
https://github.com/apache/deltaspike/blob/master/deltaspike/core/impl/src/main/java/org/apache/deltaspike/core/impl/config/EnvironmentPropertyConfigSource.java#L55
or a more fancy - but real world - one like key ->
key.substring(prefix.length()).toUpperCase(ROOT).replace('.', '_')
Romain Manni-Bucau
@rmannibucau <https://twitter.com/rmannibucau> | Blog
<https://rmannibucau.metawerx.net/> | Old Blog
<http://rmannibucau.wordpress.com> | Github <https://github.com/rmannibucau> |
LinkedIn <https://www.linkedin.com/in/rmannibucau> | Book
<https://www.packtpub.com/application-development/java-ee-8-high-performance>
2018-02-14 15:42 GMT+01:00 Ismaël Mejía <ie...@gmail.com>:
> +1 to Romain idea with Kenn's approach, we should probably reserve the
> 'beam.' namespace too because we are already using it for system
> properties in the spark runner, following along the convention of
> 'spark.*' in Apache Spark.
>
> Any ideas on how to handle this for the env vraiables case ? maybe this?
> export my.random.namespace="SomeOption=bizzle SomeOtherOption=whatever"
>
>
> On Wed, Feb 14, 2018 at 9:35 AM, Romain Manni-Bucau
> <rm...@gmail.com> wrote:
> > FYI created https://github.com/apache/beam/pull/4683 about it
> >
> >
> > Romain Manni-Bucau
> > @rmannibucau | Blog | Old Blog | Github | LinkedIn | Book
> >
> > 2018-02-13 19:21 GMT+01:00 Romain Manni-Bucau <rm...@gmail.com>:
> >>
> >> Oki, will try a PR after the classloader one then. Thanks a lot.
> >>
> >> Le 13 févr. 2018 19:14, "Lukasz Cwik" <lc...@google.com> a écrit :
> >>>
> >>> The one in Dataflow 1.x was one system property that contained all the
> >>> JSON so it wasn't exactly what you were looking for.
> >>>
> >>> On Tue, Feb 13, 2018 at 9:51 AM, Romain Manni-Bucau
> >>> <rm...@gmail.com> wrote:
> >>>>
> >>>> I like your proposal Kenneth. Perfectly fits my use case and
> deployment
> >>>> one as well - when ops configure the env without modifying the code.
> >>>>
> >>>> How do we move forward on that? Should I send a PR or do you want to
> >>>> import what was in dataflow?
> >>>>
> >>>>
> >>>> Romain Manni-Bucau
> >>>> @rmannibucau | Blog | Old Blog | Github | LinkedIn | Book
> >>>>
> >>>> 2018-02-13 18:49 GMT+01:00 Lukasz Cwik <lc...@google.com>:
> >>>>>
> >>>>> PipelineOptionsFactory.fromSystemProperties did exist in Dataflow
> 1.x
> >>>>> and was dropped for the reason that Ken mentioned.
> >>>>>
> >>>>> On Tue, Feb 13, 2018 at 9:46 AM, Kenneth Knowles <kl...@google.com>
> >>>>> wrote:
> >>>>>>
> >>>>>> Pipeline options are not global - they are a property of a single
> job.
> >>>>>> The TestPipeline reads them from a very particular system property
> because
> >>>>>> it is a special testing rule.
> >>>>>>
> >>>>>> If you want a generic way to build pipeline options from a set of
> >>>>>> system properties, it should be from an explicit prefix, not global
> >>>>>> defaults. So if a user wants to put a bunch of options into
> properties, they
> >>>>>> choose a namespace like "my.random.namespace" and everything under
> it can be
> >>>>>> interpreted as a pipelineoption:
> >>>>>>
> >>>>>> my.random.namespace.SomeOption=bizzle
> >>>>>> my.random.namespace.SomeOtherOption=whatever
> >>>>>>
> >>>>>> And you could do
> >>>>>> PipelineOptionsFactory.fromSystemProperties("my.random.namespace")
> >>>>>>
> >>>>>> We should use the full name of the option not split it with dots -
> >>>>>> dots represent hierarchy not words separation.
> >>>>>>
> >>>>>> interface FooOptions extends PipelineOptions {
> >>>>>> getSomeOption()
> >>>>>> getSomeOtherOption()
> >>>>>> }
> >>>>>>
> >>>>>> I think I can be +0.5 on this. We might also reserve a namespace
> like
> >>>>>> "beam.TestPipeline.options" and use it for specification of test
> config.
> >>>>>> Could be easier than embedding JSON in a property, in some cases.
> Easier to
> >>>>>> override little pieces for sure.
> >>>>>>
> >>>>>> Kenn
> >>>>>>
> >>>>>> On Tue, Feb 13, 2018 at 9:29 AM, Romain Manni-Bucau
> >>>>>> <rm...@gmail.com> wrote:
> >>>>>>>
> >>>>>>> makes sense, do we want beam.foo.bar -> --foo-bar conversion too?
> >>>>>>>
> >>>>>>>
> >>>>>>> Romain Manni-Bucau
> >>>>>>> @rmannibucau | Blog | Old Blog | Github | LinkedIn | Book
> >>>>>>>
> >>>>>>> 2018-02-13 18:19 GMT+01:00 Eugene Kirpichov <kirpichov@google.com
> >:
> >>>>>>>>
> >>>>>>>> Neutral about this one: haven't seen a case where this was needed,
> >>>>>>>> but don't see anything wrong with it either. One thing I'd
> recommend if you
> >>>>>>>> go through with it, extract from system properties under "beam."
> rather than
> >>>>>>>> all of them, to avoid clashes.
> >>>>>>>>
> >>>>>>>>
> >>>>>>>> On Tue, Feb 13, 2018, 7:53 AM Jean-Baptiste Onofré <
> jb@nanthrax.net>
> >>>>>>>> wrote:
> >>>>>>>>>
> >>>>>>>>> Hi Romain,
> >>>>>>>>>
> >>>>>>>>> it sounds interesting to me, and doesn't break anything, so +1
> from
> >>>>>>>>> my side.
> >>>>>>>>>
> >>>>>>>>> Regards
> >>>>>>>>> JB
> >>>>>>>>>
> >>>>>>>>> On 02/13/2018 03:42 PM, Romain Manni-Bucau wrote:
> >>>>>>>>> > Hi guys,
> >>>>>>>>> >
> >>>>>>>>> > there are hacks in beam testing code to read the args from a
> >>>>>>>>> > system property but
> >>>>>>>>> > I wonder if we shouldnt add a
> >>>>>>>>> > PipelineOptionsFactory.fromSystemProperties().
> >>>>>>>>> >
> >>>>>>>>> > It would iterate over the system properties and take all
> >>>>>>>>> > --xxx=foo as potential
> >>>>>>>>> > argument it tries to bind.
> >>>>>>>>> >
> >>>>>>>>> > Rational behind that is to enable users to wrap the pipeline
> API
> >>>>>>>>> > but still
> >>>>>>>>> > expose the pipeline options to end users for advanced cases.
> >>>>>>>>> >
> >>>>>>>>> > Any discussion on this kind of usages already? What do you
> think
> >>>>>>>>> > of this proposal?
> >>>>>>>>> >
> >>>>>>>>> > Side note: we can think about a fromEnv() too.
> >>>>>>>>> >
> >>>>>>>>> > Romain Manni-Bucau
> >>>>>>>>> > @rmannibucau <https://twitter.com/rmannibucau> | Blog
> >>>>>>>>> > <https://rmannibucau.metawerx.net/> | Old Blog
> >>>>>>>>> > <http://rmannibucau.wordpress.com> | Github
> >>>>>>>>> > <https://github.com/rmannibucau> |
> >>>>>>>>> > LinkedIn <https://www.linkedin.com/in/rmannibucau> | Book
> >>>>>>>>> >
> >>>>>>>>> > <https://www.packtpub.com/application-development/java-
> ee-8-high-performance>
> >>>>>>>>>
> >>>>>>>>> --
> >>>>>>>>> Jean-Baptiste Onofré
> >>>>>>>>> jbonofre@apache.org
> >>>>>>>>> http://blog.nanthrax.net
> >>>>>>>>> Talend - http://www.talend.com
> >>>>>>>
> >>>>>>>
> >>>>>>
> >>>>>
> >>>>
> >>>
> >
>
Re: PipelineOptions fromSystemProps?
Posted by Ismaël Mejía <ie...@gmail.com>.
+1 to Romain idea with Kenn's approach, we should probably reserve the
'beam.' namespace too because we are already using it for system
properties in the spark runner, following along the convention of
'spark.*' in Apache Spark.
Any ideas on how to handle this for the env vraiables case ? maybe this?
export my.random.namespace="SomeOption=bizzle SomeOtherOption=whatever"
On Wed, Feb 14, 2018 at 9:35 AM, Romain Manni-Bucau
<rm...@gmail.com> wrote:
> FYI created https://github.com/apache/beam/pull/4683 about it
>
>
> Romain Manni-Bucau
> @rmannibucau | Blog | Old Blog | Github | LinkedIn | Book
>
> 2018-02-13 19:21 GMT+01:00 Romain Manni-Bucau <rm...@gmail.com>:
>>
>> Oki, will try a PR after the classloader one then. Thanks a lot.
>>
>> Le 13 févr. 2018 19:14, "Lukasz Cwik" <lc...@google.com> a écrit :
>>>
>>> The one in Dataflow 1.x was one system property that contained all the
>>> JSON so it wasn't exactly what you were looking for.
>>>
>>> On Tue, Feb 13, 2018 at 9:51 AM, Romain Manni-Bucau
>>> <rm...@gmail.com> wrote:
>>>>
>>>> I like your proposal Kenneth. Perfectly fits my use case and deployment
>>>> one as well - when ops configure the env without modifying the code.
>>>>
>>>> How do we move forward on that? Should I send a PR or do you want to
>>>> import what was in dataflow?
>>>>
>>>>
>>>> Romain Manni-Bucau
>>>> @rmannibucau | Blog | Old Blog | Github | LinkedIn | Book
>>>>
>>>> 2018-02-13 18:49 GMT+01:00 Lukasz Cwik <lc...@google.com>:
>>>>>
>>>>> PipelineOptionsFactory.fromSystemProperties did exist in Dataflow 1.x
>>>>> and was dropped for the reason that Ken mentioned.
>>>>>
>>>>> On Tue, Feb 13, 2018 at 9:46 AM, Kenneth Knowles <kl...@google.com>
>>>>> wrote:
>>>>>>
>>>>>> Pipeline options are not global - they are a property of a single job.
>>>>>> The TestPipeline reads them from a very particular system property because
>>>>>> it is a special testing rule.
>>>>>>
>>>>>> If you want a generic way to build pipeline options from a set of
>>>>>> system properties, it should be from an explicit prefix, not global
>>>>>> defaults. So if a user wants to put a bunch of options into properties, they
>>>>>> choose a namespace like "my.random.namespace" and everything under it can be
>>>>>> interpreted as a pipelineoption:
>>>>>>
>>>>>> my.random.namespace.SomeOption=bizzle
>>>>>> my.random.namespace.SomeOtherOption=whatever
>>>>>>
>>>>>> And you could do
>>>>>> PipelineOptionsFactory.fromSystemProperties("my.random.namespace")
>>>>>>
>>>>>> We should use the full name of the option not split it with dots -
>>>>>> dots represent hierarchy not words separation.
>>>>>>
>>>>>> interface FooOptions extends PipelineOptions {
>>>>>> getSomeOption()
>>>>>> getSomeOtherOption()
>>>>>> }
>>>>>>
>>>>>> I think I can be +0.5 on this. We might also reserve a namespace like
>>>>>> "beam.TestPipeline.options" and use it for specification of test config.
>>>>>> Could be easier than embedding JSON in a property, in some cases. Easier to
>>>>>> override little pieces for sure.
>>>>>>
>>>>>> Kenn
>>>>>>
>>>>>> On Tue, Feb 13, 2018 at 9:29 AM, Romain Manni-Bucau
>>>>>> <rm...@gmail.com> wrote:
>>>>>>>
>>>>>>> makes sense, do we want beam.foo.bar -> --foo-bar conversion too?
>>>>>>>
>>>>>>>
>>>>>>> Romain Manni-Bucau
>>>>>>> @rmannibucau | Blog | Old Blog | Github | LinkedIn | Book
>>>>>>>
>>>>>>> 2018-02-13 18:19 GMT+01:00 Eugene Kirpichov <ki...@google.com>:
>>>>>>>>
>>>>>>>> Neutral about this one: haven't seen a case where this was needed,
>>>>>>>> but don't see anything wrong with it either. One thing I'd recommend if you
>>>>>>>> go through with it, extract from system properties under "beam." rather than
>>>>>>>> all of them, to avoid clashes.
>>>>>>>>
>>>>>>>>
>>>>>>>> On Tue, Feb 13, 2018, 7:53 AM Jean-Baptiste Onofré <jb...@nanthrax.net>
>>>>>>>> wrote:
>>>>>>>>>
>>>>>>>>> Hi Romain,
>>>>>>>>>
>>>>>>>>> it sounds interesting to me, and doesn't break anything, so +1 from
>>>>>>>>> my side.
>>>>>>>>>
>>>>>>>>> Regards
>>>>>>>>> JB
>>>>>>>>>
>>>>>>>>> On 02/13/2018 03:42 PM, Romain Manni-Bucau wrote:
>>>>>>>>> > Hi guys,
>>>>>>>>> >
>>>>>>>>> > there are hacks in beam testing code to read the args from a
>>>>>>>>> > system property but
>>>>>>>>> > I wonder if we shouldnt add a
>>>>>>>>> > PipelineOptionsFactory.fromSystemProperties().
>>>>>>>>> >
>>>>>>>>> > It would iterate over the system properties and take all
>>>>>>>>> > --xxx=foo as potential
>>>>>>>>> > argument it tries to bind.
>>>>>>>>> >
>>>>>>>>> > Rational behind that is to enable users to wrap the pipeline API
>>>>>>>>> > but still
>>>>>>>>> > expose the pipeline options to end users for advanced cases.
>>>>>>>>> >
>>>>>>>>> > Any discussion on this kind of usages already? What do you think
>>>>>>>>> > of this proposal?
>>>>>>>>> >
>>>>>>>>> > Side note: we can think about a fromEnv() too.
>>>>>>>>> >
>>>>>>>>> > Romain Manni-Bucau
>>>>>>>>> > @rmannibucau <https://twitter.com/rmannibucau> | Blog
>>>>>>>>> > <https://rmannibucau.metawerx.net/> | Old Blog
>>>>>>>>> > <http://rmannibucau.wordpress.com> | Github
>>>>>>>>> > <https://github.com/rmannibucau> |
>>>>>>>>> > LinkedIn <https://www.linkedin.com/in/rmannibucau> | Book
>>>>>>>>> >
>>>>>>>>> > <https://www.packtpub.com/application-development/java-ee-8-high-performance>
>>>>>>>>>
>>>>>>>>> --
>>>>>>>>> Jean-Baptiste Onofré
>>>>>>>>> jbonofre@apache.org
>>>>>>>>> http://blog.nanthrax.net
>>>>>>>>> Talend - http://www.talend.com
>>>>>>>
>>>>>>>
>>>>>>
>>>>>
>>>>
>>>
>
Re: PipelineOptions fromSystemProps?
Posted by Romain Manni-Bucau <rm...@gmail.com>.
FYI created https://github.com/apache/beam/pull/4683 about it
Romain Manni-Bucau
@rmannibucau <https://twitter.com/rmannibucau> | Blog
<https://rmannibucau.metawerx.net/> | Old Blog
<http://rmannibucau.wordpress.com> | Github <https://github.com/rmannibucau> |
LinkedIn <https://www.linkedin.com/in/rmannibucau> | Book
<https://www.packtpub.com/application-development/java-ee-8-high-performance>
2018-02-13 19:21 GMT+01:00 Romain Manni-Bucau <rm...@gmail.com>:
> Oki, will try a PR after the classloader one then. Thanks a lot.
>
> Le 13 févr. 2018 19:14, "Lukasz Cwik" <lc...@google.com> a écrit :
>
>> The one in Dataflow 1.x was one system property that contained all the
>> JSON so it wasn't exactly what you were looking for.
>>
>> On Tue, Feb 13, 2018 at 9:51 AM, Romain Manni-Bucau <
>> rmannibucau@gmail.com> wrote:
>>
>>> I like your proposal Kenneth. Perfectly fits my use case and deployment
>>> one as well - when ops configure the env without modifying the code.
>>>
>>> How do we move forward on that? Should I send a PR or do you want to
>>> import what was in dataflow?
>>>
>>>
>>> Romain Manni-Bucau
>>> @rmannibucau <https://twitter.com/rmannibucau> | Blog
>>> <https://rmannibucau.metawerx.net/> | Old Blog
>>> <http://rmannibucau.wordpress.com> | Github
>>> <https://github.com/rmannibucau> | LinkedIn
>>> <https://www.linkedin.com/in/rmannibucau> | Book
>>> <https://www.packtpub.com/application-development/java-ee-8-high-performance>
>>>
>>> 2018-02-13 18:49 GMT+01:00 Lukasz Cwik <lc...@google.com>:
>>>
>>>> PipelineOptionsFactory.fromSystemProperties did exist in Dataflow 1.x
>>>> and was dropped for the reason that Ken mentioned.
>>>>
>>>> On Tue, Feb 13, 2018 at 9:46 AM, Kenneth Knowles <kl...@google.com>
>>>> wrote:
>>>>
>>>>> Pipeline options are not global - they are a property of a single job.
>>>>> The TestPipeline reads them from a very particular system property because
>>>>> it is a special testing rule.
>>>>>
>>>>> If you want a generic way to build pipeline options from a set of
>>>>> system properties, it should be from an explicit prefix, not global
>>>>> defaults. So if a user wants to put a bunch of options into properties,
>>>>> they choose a namespace like "my.random.namespace" and everything under it
>>>>> can be interpreted as a pipelineoption:
>>>>>
>>>>> my.random.namespace.SomeOption=bizzle
>>>>> my.random.namespace.SomeOtherOption=whatever
>>>>>
>>>>> And you could do PipelineOptionsFactory.fromSys
>>>>> temProperties("my.random.namespace")
>>>>>
>>>>> We should use the full name of the option not split it with dots -
>>>>> dots represent hierarchy not words separation.
>>>>>
>>>>> interface FooOptions extends PipelineOptions {
>>>>> getSomeOption()
>>>>> getSomeOtherOption()
>>>>> }
>>>>>
>>>>> I think I can be +0.5 on this. We might also reserve a namespace like
>>>>> "beam.TestPipeline.options" and use it for specification of test config.
>>>>> Could be easier than embedding JSON in a property, in some cases. Easier to
>>>>> override little pieces for sure.
>>>>>
>>>>> Kenn
>>>>>
>>>>> On Tue, Feb 13, 2018 at 9:29 AM, Romain Manni-Bucau <
>>>>> rmannibucau@gmail.com> wrote:
>>>>>
>>>>>> makes sense, do we want beam.foo.bar -> --foo-bar conversion too?
>>>>>>
>>>>>>
>>>>>> Romain Manni-Bucau
>>>>>> @rmannibucau <https://twitter.com/rmannibucau> | Blog
>>>>>> <https://rmannibucau.metawerx.net/> | Old Blog
>>>>>> <http://rmannibucau.wordpress.com> | Github
>>>>>> <https://github.com/rmannibucau> | LinkedIn
>>>>>> <https://www.linkedin.com/in/rmannibucau> | Book
>>>>>> <https://www.packtpub.com/application-development/java-ee-8-high-performance>
>>>>>>
>>>>>> 2018-02-13 18:19 GMT+01:00 Eugene Kirpichov <ki...@google.com>:
>>>>>>
>>>>>>> Neutral about this one: haven't seen a case where this was needed,
>>>>>>> but don't see anything wrong with it either. One thing I'd recommend if you
>>>>>>> go through with it, extract from system properties under "beam." rather
>>>>>>> than all of them, to avoid clashes.
>>>>>>>
>>>>>>> On Tue, Feb 13, 2018, 7:53 AM Jean-Baptiste Onofré <jb...@nanthrax.net>
>>>>>>> wrote:
>>>>>>>
>>>>>>>> Hi Romain,
>>>>>>>>
>>>>>>>> it sounds interesting to me, and doesn't break anything, so +1 from
>>>>>>>> my side.
>>>>>>>>
>>>>>>>> Regards
>>>>>>>> JB
>>>>>>>>
>>>>>>>> On 02/13/2018 03:42 PM, Romain Manni-Bucau wrote:
>>>>>>>> > Hi guys,
>>>>>>>> >
>>>>>>>> > there are hacks in beam testing code to read the args from a
>>>>>>>> system property but
>>>>>>>> > I wonder if we shouldnt add a PipelineOptionsFactory.fromSys
>>>>>>>> temProperties().
>>>>>>>> >
>>>>>>>> > It would iterate over the system properties and take all
>>>>>>>> --xxx=foo as potential
>>>>>>>> > argument it tries to bind.
>>>>>>>> >
>>>>>>>> > Rational behind that is to enable users to wrap the pipeline API
>>>>>>>> but still
>>>>>>>> > expose the pipeline options to end users for advanced cases.
>>>>>>>> >
>>>>>>>> > Any discussion on this kind of usages already? What do you think
>>>>>>>> of this proposal?
>>>>>>>> >
>>>>>>>> > Side note: we can think about a fromEnv() too.
>>>>>>>> >
>>>>>>>> > Romain Manni-Bucau
>>>>>>>> > @rmannibucau <https://twitter.com/rmannibucau> | Blog
>>>>>>>> > <https://rmannibucau.metawerx.net/> | Old Blog
>>>>>>>> > <http://rmannibucau.wordpress.com> | Github <
>>>>>>>> https://github.com/rmannibucau> |
>>>>>>>> > LinkedIn <https://www.linkedin.com/in/rmannibucau> | Book
>>>>>>>> > <https://www.packtpub.com/application-development/java-ee-8-
>>>>>>>> high-performance>
>>>>>>>>
>>>>>>>> --
>>>>>>>> Jean-Baptiste Onofré
>>>>>>>> jbonofre@apache.org
>>>>>>>> http://blog.nanthrax.net
>>>>>>>> Talend - http://www.talend.com
>>>>>>>>
>>>>>>>
>>>>>>
>>>>>
>>>>
>>>
>>
Re: PipelineOptions fromSystemProps?
Posted by Romain Manni-Bucau <rm...@gmail.com>.
Oki, will try a PR after the classloader one then. Thanks a lot.
Le 13 févr. 2018 19:14, "Lukasz Cwik" <lc...@google.com> a écrit :
> The one in Dataflow 1.x was one system property that contained all the
> JSON so it wasn't exactly what you were looking for.
>
> On Tue, Feb 13, 2018 at 9:51 AM, Romain Manni-Bucau <rmannibucau@gmail.com
> > wrote:
>
>> I like your proposal Kenneth. Perfectly fits my use case and deployment
>> one as well - when ops configure the env without modifying the code.
>>
>> How do we move forward on that? Should I send a PR or do you want to
>> import what was in dataflow?
>>
>>
>> Romain Manni-Bucau
>> @rmannibucau <https://twitter.com/rmannibucau> | Blog
>> <https://rmannibucau.metawerx.net/> | Old Blog
>> <http://rmannibucau.wordpress.com> | Github
>> <https://github.com/rmannibucau> | LinkedIn
>> <https://www.linkedin.com/in/rmannibucau> | Book
>> <https://www.packtpub.com/application-development/java-ee-8-high-performance>
>>
>> 2018-02-13 18:49 GMT+01:00 Lukasz Cwik <lc...@google.com>:
>>
>>> PipelineOptionsFactory.fromSystemProperties did exist in Dataflow 1.x
>>> and was dropped for the reason that Ken mentioned.
>>>
>>> On Tue, Feb 13, 2018 at 9:46 AM, Kenneth Knowles <kl...@google.com> wrote:
>>>
>>>> Pipeline options are not global - they are a property of a single job.
>>>> The TestPipeline reads them from a very particular system property because
>>>> it is a special testing rule.
>>>>
>>>> If you want a generic way to build pipeline options from a set of
>>>> system properties, it should be from an explicit prefix, not global
>>>> defaults. So if a user wants to put a bunch of options into properties,
>>>> they choose a namespace like "my.random.namespace" and everything under it
>>>> can be interpreted as a pipelineoption:
>>>>
>>>> my.random.namespace.SomeOption=bizzle
>>>> my.random.namespace.SomeOtherOption=whatever
>>>>
>>>> And you could do PipelineOptionsFactory.fromSys
>>>> temProperties("my.random.namespace")
>>>>
>>>> We should use the full name of the option not split it with dots - dots
>>>> represent hierarchy not words separation.
>>>>
>>>> interface FooOptions extends PipelineOptions {
>>>> getSomeOption()
>>>> getSomeOtherOption()
>>>> }
>>>>
>>>> I think I can be +0.5 on this. We might also reserve a namespace like
>>>> "beam.TestPipeline.options" and use it for specification of test config.
>>>> Could be easier than embedding JSON in a property, in some cases. Easier to
>>>> override little pieces for sure.
>>>>
>>>> Kenn
>>>>
>>>> On Tue, Feb 13, 2018 at 9:29 AM, Romain Manni-Bucau <
>>>> rmannibucau@gmail.com> wrote:
>>>>
>>>>> makes sense, do we want beam.foo.bar -> --foo-bar conversion too?
>>>>>
>>>>>
>>>>> Romain Manni-Bucau
>>>>> @rmannibucau <https://twitter.com/rmannibucau> | Blog
>>>>> <https://rmannibucau.metawerx.net/> | Old Blog
>>>>> <http://rmannibucau.wordpress.com> | Github
>>>>> <https://github.com/rmannibucau> | LinkedIn
>>>>> <https://www.linkedin.com/in/rmannibucau> | Book
>>>>> <https://www.packtpub.com/application-development/java-ee-8-high-performance>
>>>>>
>>>>> 2018-02-13 18:19 GMT+01:00 Eugene Kirpichov <ki...@google.com>:
>>>>>
>>>>>> Neutral about this one: haven't seen a case where this was needed,
>>>>>> but don't see anything wrong with it either. One thing I'd recommend if you
>>>>>> go through with it, extract from system properties under "beam." rather
>>>>>> than all of them, to avoid clashes.
>>>>>>
>>>>>> On Tue, Feb 13, 2018, 7:53 AM Jean-Baptiste Onofré <jb...@nanthrax.net>
>>>>>> wrote:
>>>>>>
>>>>>>> Hi Romain,
>>>>>>>
>>>>>>> it sounds interesting to me, and doesn't break anything, so +1 from
>>>>>>> my side.
>>>>>>>
>>>>>>> Regards
>>>>>>> JB
>>>>>>>
>>>>>>> On 02/13/2018 03:42 PM, Romain Manni-Bucau wrote:
>>>>>>> > Hi guys,
>>>>>>> >
>>>>>>> > there are hacks in beam testing code to read the args from a
>>>>>>> system property but
>>>>>>> > I wonder if we shouldnt add a PipelineOptionsFactory.fromSys
>>>>>>> temProperties().
>>>>>>> >
>>>>>>> > It would iterate over the system properties and take all --xxx=foo
>>>>>>> as potential
>>>>>>> > argument it tries to bind.
>>>>>>> >
>>>>>>> > Rational behind that is to enable users to wrap the pipeline API
>>>>>>> but still
>>>>>>> > expose the pipeline options to end users for advanced cases.
>>>>>>> >
>>>>>>> > Any discussion on this kind of usages already? What do you think
>>>>>>> of this proposal?
>>>>>>> >
>>>>>>> > Side note: we can think about a fromEnv() too.
>>>>>>> >
>>>>>>> > Romain Manni-Bucau
>>>>>>> > @rmannibucau <https://twitter.com/rmannibucau> | Blog
>>>>>>> > <https://rmannibucau.metawerx.net/> | Old Blog
>>>>>>> > <http://rmannibucau.wordpress.com> | Github <
>>>>>>> https://github.com/rmannibucau> |
>>>>>>> > LinkedIn <https://www.linkedin.com/in/rmannibucau> | Book
>>>>>>> > <https://www.packtpub.com/application-development/java-ee-8-
>>>>>>> high-performance>
>>>>>>>
>>>>>>> --
>>>>>>> Jean-Baptiste Onofré
>>>>>>> jbonofre@apache.org
>>>>>>> http://blog.nanthrax.net
>>>>>>> Talend - http://www.talend.com
>>>>>>>
>>>>>>
>>>>>
>>>>
>>>
>>
>
Re: PipelineOptions fromSystemProps?
Posted by Lukasz Cwik <lc...@google.com>.
The one in Dataflow 1.x was one system property that contained all the JSON
so it wasn't exactly what you were looking for.
On Tue, Feb 13, 2018 at 9:51 AM, Romain Manni-Bucau <rm...@gmail.com>
wrote:
> I like your proposal Kenneth. Perfectly fits my use case and deployment
> one as well - when ops configure the env without modifying the code.
>
> How do we move forward on that? Should I send a PR or do you want to
> import what was in dataflow?
>
>
> Romain Manni-Bucau
> @rmannibucau <https://twitter.com/rmannibucau> | Blog
> <https://rmannibucau.metawerx.net/> | Old Blog
> <http://rmannibucau.wordpress.com> | Github
> <https://github.com/rmannibucau> | LinkedIn
> <https://www.linkedin.com/in/rmannibucau> | Book
> <https://www.packtpub.com/application-development/java-ee-8-high-performance>
>
> 2018-02-13 18:49 GMT+01:00 Lukasz Cwik <lc...@google.com>:
>
>> PipelineOptionsFactory.fromSystemProperties did exist in Dataflow 1.x
>> and was dropped for the reason that Ken mentioned.
>>
>> On Tue, Feb 13, 2018 at 9:46 AM, Kenneth Knowles <kl...@google.com> wrote:
>>
>>> Pipeline options are not global - they are a property of a single job.
>>> The TestPipeline reads them from a very particular system property because
>>> it is a special testing rule.
>>>
>>> If you want a generic way to build pipeline options from a set of system
>>> properties, it should be from an explicit prefix, not global defaults. So
>>> if a user wants to put a bunch of options into properties, they choose a
>>> namespace like "my.random.namespace" and everything under it can be
>>> interpreted as a pipelineoption:
>>>
>>> my.random.namespace.SomeOption=bizzle
>>> my.random.namespace.SomeOtherOption=whatever
>>>
>>> And you could do PipelineOptionsFactory.fromSys
>>> temProperties("my.random.namespace")
>>>
>>> We should use the full name of the option not split it with dots - dots
>>> represent hierarchy not words separation.
>>>
>>> interface FooOptions extends PipelineOptions {
>>> getSomeOption()
>>> getSomeOtherOption()
>>> }
>>>
>>> I think I can be +0.5 on this. We might also reserve a namespace like
>>> "beam.TestPipeline.options" and use it for specification of test config.
>>> Could be easier than embedding JSON in a property, in some cases. Easier to
>>> override little pieces for sure.
>>>
>>> Kenn
>>>
>>> On Tue, Feb 13, 2018 at 9:29 AM, Romain Manni-Bucau <
>>> rmannibucau@gmail.com> wrote:
>>>
>>>> makes sense, do we want beam.foo.bar -> --foo-bar conversion too?
>>>>
>>>>
>>>> Romain Manni-Bucau
>>>> @rmannibucau <https://twitter.com/rmannibucau> | Blog
>>>> <https://rmannibucau.metawerx.net/> | Old Blog
>>>> <http://rmannibucau.wordpress.com> | Github
>>>> <https://github.com/rmannibucau> | LinkedIn
>>>> <https://www.linkedin.com/in/rmannibucau> | Book
>>>> <https://www.packtpub.com/application-development/java-ee-8-high-performance>
>>>>
>>>> 2018-02-13 18:19 GMT+01:00 Eugene Kirpichov <ki...@google.com>:
>>>>
>>>>> Neutral about this one: haven't seen a case where this was needed, but
>>>>> don't see anything wrong with it either. One thing I'd recommend if you go
>>>>> through with it, extract from system properties under "beam." rather than
>>>>> all of them, to avoid clashes.
>>>>>
>>>>> On Tue, Feb 13, 2018, 7:53 AM Jean-Baptiste Onofré <jb...@nanthrax.net>
>>>>> wrote:
>>>>>
>>>>>> Hi Romain,
>>>>>>
>>>>>> it sounds interesting to me, and doesn't break anything, so +1 from
>>>>>> my side.
>>>>>>
>>>>>> Regards
>>>>>> JB
>>>>>>
>>>>>> On 02/13/2018 03:42 PM, Romain Manni-Bucau wrote:
>>>>>> > Hi guys,
>>>>>> >
>>>>>> > there are hacks in beam testing code to read the args from a system
>>>>>> property but
>>>>>> > I wonder if we shouldnt add a PipelineOptionsFactory.fromSys
>>>>>> temProperties().
>>>>>> >
>>>>>> > It would iterate over the system properties and take all --xxx=foo
>>>>>> as potential
>>>>>> > argument it tries to bind.
>>>>>> >
>>>>>> > Rational behind that is to enable users to wrap the pipeline API
>>>>>> but still
>>>>>> > expose the pipeline options to end users for advanced cases.
>>>>>> >
>>>>>> > Any discussion on this kind of usages already? What do you think of
>>>>>> this proposal?
>>>>>> >
>>>>>> > Side note: we can think about a fromEnv() too.
>>>>>> >
>>>>>> > Romain Manni-Bucau
>>>>>> > @rmannibucau <https://twitter.com/rmannibucau> | Blog
>>>>>> > <https://rmannibucau.metawerx.net/> | Old Blog
>>>>>> > <http://rmannibucau.wordpress.com> | Github <
>>>>>> https://github.com/rmannibucau> |
>>>>>> > LinkedIn <https://www.linkedin.com/in/rmannibucau> | Book
>>>>>> > <https://www.packtpub.com/application-development/java-ee-8-
>>>>>> high-performance>
>>>>>>
>>>>>> --
>>>>>> Jean-Baptiste Onofré
>>>>>> jbonofre@apache.org
>>>>>> http://blog.nanthrax.net
>>>>>> Talend - http://www.talend.com
>>>>>>
>>>>>
>>>>
>>>
>>
>
Re: PipelineOptions fromSystemProps?
Posted by Romain Manni-Bucau <rm...@gmail.com>.
I like your proposal Kenneth. Perfectly fits my use case and deployment one
as well - when ops configure the env without modifying the code.
How do we move forward on that? Should I send a PR or do you want to import
what was in dataflow?
Romain Manni-Bucau
@rmannibucau <https://twitter.com/rmannibucau> | Blog
<https://rmannibucau.metawerx.net/> | Old Blog
<http://rmannibucau.wordpress.com> | Github <https://github.com/rmannibucau> |
LinkedIn <https://www.linkedin.com/in/rmannibucau> | Book
<https://www.packtpub.com/application-development/java-ee-8-high-performance>
2018-02-13 18:49 GMT+01:00 Lukasz Cwik <lc...@google.com>:
> PipelineOptionsFactory.fromSystemProperties did exist in Dataflow 1.x and
> was dropped for the reason that Ken mentioned.
>
> On Tue, Feb 13, 2018 at 9:46 AM, Kenneth Knowles <kl...@google.com> wrote:
>
>> Pipeline options are not global - they are a property of a single job.
>> The TestPipeline reads them from a very particular system property because
>> it is a special testing rule.
>>
>> If you want a generic way to build pipeline options from a set of system
>> properties, it should be from an explicit prefix, not global defaults. So
>> if a user wants to put a bunch of options into properties, they choose a
>> namespace like "my.random.namespace" and everything under it can be
>> interpreted as a pipelineoption:
>>
>> my.random.namespace.SomeOption=bizzle
>> my.random.namespace.SomeOtherOption=whatever
>>
>> And you could do PipelineOptionsFactory.fromSystemProperties("my.random.
>> namespace")
>>
>> We should use the full name of the option not split it with dots - dots
>> represent hierarchy not words separation.
>>
>> interface FooOptions extends PipelineOptions {
>> getSomeOption()
>> getSomeOtherOption()
>> }
>>
>> I think I can be +0.5 on this. We might also reserve a namespace like
>> "beam.TestPipeline.options" and use it for specification of test config.
>> Could be easier than embedding JSON in a property, in some cases. Easier to
>> override little pieces for sure.
>>
>> Kenn
>>
>> On Tue, Feb 13, 2018 at 9:29 AM, Romain Manni-Bucau <
>> rmannibucau@gmail.com> wrote:
>>
>>> makes sense, do we want beam.foo.bar -> --foo-bar conversion too?
>>>
>>>
>>> Romain Manni-Bucau
>>> @rmannibucau <https://twitter.com/rmannibucau> | Blog
>>> <https://rmannibucau.metawerx.net/> | Old Blog
>>> <http://rmannibucau.wordpress.com> | Github
>>> <https://github.com/rmannibucau> | LinkedIn
>>> <https://www.linkedin.com/in/rmannibucau> | Book
>>> <https://www.packtpub.com/application-development/java-ee-8-high-performance>
>>>
>>> 2018-02-13 18:19 GMT+01:00 Eugene Kirpichov <ki...@google.com>:
>>>
>>>> Neutral about this one: haven't seen a case where this was needed, but
>>>> don't see anything wrong with it either. One thing I'd recommend if you go
>>>> through with it, extract from system properties under "beam." rather than
>>>> all of them, to avoid clashes.
>>>>
>>>> On Tue, Feb 13, 2018, 7:53 AM Jean-Baptiste Onofré <jb...@nanthrax.net>
>>>> wrote:
>>>>
>>>>> Hi Romain,
>>>>>
>>>>> it sounds interesting to me, and doesn't break anything, so +1 from my
>>>>> side.
>>>>>
>>>>> Regards
>>>>> JB
>>>>>
>>>>> On 02/13/2018 03:42 PM, Romain Manni-Bucau wrote:
>>>>> > Hi guys,
>>>>> >
>>>>> > there are hacks in beam testing code to read the args from a system
>>>>> property but
>>>>> > I wonder if we shouldnt add a PipelineOptionsFactory.fromSys
>>>>> temProperties().
>>>>> >
>>>>> > It would iterate over the system properties and take all --xxx=foo
>>>>> as potential
>>>>> > argument it tries to bind.
>>>>> >
>>>>> > Rational behind that is to enable users to wrap the pipeline API but
>>>>> still
>>>>> > expose the pipeline options to end users for advanced cases.
>>>>> >
>>>>> > Any discussion on this kind of usages already? What do you think of
>>>>> this proposal?
>>>>> >
>>>>> > Side note: we can think about a fromEnv() too.
>>>>> >
>>>>> > Romain Manni-Bucau
>>>>> > @rmannibucau <https://twitter.com/rmannibucau> | Blog
>>>>> > <https://rmannibucau.metawerx.net/> | Old Blog
>>>>> > <http://rmannibucau.wordpress.com> | Github <
>>>>> https://github.com/rmannibucau> |
>>>>> > LinkedIn <https://www.linkedin.com/in/rmannibucau> | Book
>>>>> > <https://www.packtpub.com/application-development/java-ee-8-
>>>>> high-performance>
>>>>>
>>>>> --
>>>>> Jean-Baptiste Onofré
>>>>> jbonofre@apache.org
>>>>> http://blog.nanthrax.net
>>>>> Talend - http://www.talend.com
>>>>>
>>>>
>>>
>>
>
Re: PipelineOptions fromSystemProps?
Posted by Lukasz Cwik <lc...@google.com>.
PipelineOptionsFactory.fromSystemProperties did exist in Dataflow 1.x and
was dropped for the reason that Ken mentioned.
On Tue, Feb 13, 2018 at 9:46 AM, Kenneth Knowles <kl...@google.com> wrote:
> Pipeline options are not global - they are a property of a single job. The
> TestPipeline reads them from a very particular system property because it
> is a special testing rule.
>
> If you want a generic way to build pipeline options from a set of system
> properties, it should be from an explicit prefix, not global defaults. So
> if a user wants to put a bunch of options into properties, they choose a
> namespace like "my.random.namespace" and everything under it can be
> interpreted as a pipelineoption:
>
> my.random.namespace.SomeOption=bizzle
> my.random.namespace.SomeOtherOption=whatever
>
> And you could do PipelineOptionsFactory.fromSystemProperties("my.
> random.namespace")
>
> We should use the full name of the option not split it with dots - dots
> represent hierarchy not words separation.
>
> interface FooOptions extends PipelineOptions {
> getSomeOption()
> getSomeOtherOption()
> }
>
> I think I can be +0.5 on this. We might also reserve a namespace like
> "beam.TestPipeline.options" and use it for specification of test config.
> Could be easier than embedding JSON in a property, in some cases. Easier to
> override little pieces for sure.
>
> Kenn
>
> On Tue, Feb 13, 2018 at 9:29 AM, Romain Manni-Bucau <rmannibucau@gmail.com
> > wrote:
>
>> makes sense, do we want beam.foo.bar -> --foo-bar conversion too?
>>
>>
>> Romain Manni-Bucau
>> @rmannibucau <https://twitter.com/rmannibucau> | Blog
>> <https://rmannibucau.metawerx.net/> | Old Blog
>> <http://rmannibucau.wordpress.com> | Github
>> <https://github.com/rmannibucau> | LinkedIn
>> <https://www.linkedin.com/in/rmannibucau> | Book
>> <https://www.packtpub.com/application-development/java-ee-8-high-performance>
>>
>> 2018-02-13 18:19 GMT+01:00 Eugene Kirpichov <ki...@google.com>:
>>
>>> Neutral about this one: haven't seen a case where this was needed, but
>>> don't see anything wrong with it either. One thing I'd recommend if you go
>>> through with it, extract from system properties under "beam." rather than
>>> all of them, to avoid clashes.
>>>
>>> On Tue, Feb 13, 2018, 7:53 AM Jean-Baptiste Onofré <jb...@nanthrax.net>
>>> wrote:
>>>
>>>> Hi Romain,
>>>>
>>>> it sounds interesting to me, and doesn't break anything, so +1 from my
>>>> side.
>>>>
>>>> Regards
>>>> JB
>>>>
>>>> On 02/13/2018 03:42 PM, Romain Manni-Bucau wrote:
>>>> > Hi guys,
>>>> >
>>>> > there are hacks in beam testing code to read the args from a system
>>>> property but
>>>> > I wonder if we shouldnt add a PipelineOptionsFactory.fromSys
>>>> temProperties().
>>>> >
>>>> > It would iterate over the system properties and take all --xxx=foo as
>>>> potential
>>>> > argument it tries to bind.
>>>> >
>>>> > Rational behind that is to enable users to wrap the pipeline API but
>>>> still
>>>> > expose the pipeline options to end users for advanced cases.
>>>> >
>>>> > Any discussion on this kind of usages already? What do you think of
>>>> this proposal?
>>>> >
>>>> > Side note: we can think about a fromEnv() too.
>>>> >
>>>> > Romain Manni-Bucau
>>>> > @rmannibucau <https://twitter.com/rmannibucau> | Blog
>>>> > <https://rmannibucau.metawerx.net/> | Old Blog
>>>> > <http://rmannibucau.wordpress.com> | Github <
>>>> https://github.com/rmannibucau> |
>>>> > LinkedIn <https://www.linkedin.com/in/rmannibucau> | Book
>>>> > <https://www.packtpub.com/application-development/java-ee-8-
>>>> high-performance>
>>>>
>>>> --
>>>> Jean-Baptiste Onofré
>>>> jbonofre@apache.org
>>>> http://blog.nanthrax.net
>>>> Talend - http://www.talend.com
>>>>
>>>
>>
>
Re: PipelineOptions fromSystemProps?
Posted by Kenneth Knowles <kl...@google.com>.
Pipeline options are not global - they are a property of a single job. The
TestPipeline reads them from a very particular system property because it
is a special testing rule.
If you want a generic way to build pipeline options from a set of system
properties, it should be from an explicit prefix, not global defaults. So
if a user wants to put a bunch of options into properties, they choose a
namespace like "my.random.namespace" and everything under it can be
interpreted as a pipelineoption:
my.random.namespace.SomeOption=bizzle
my.random.namespace.SomeOtherOption=whatever
And you could do
PipelineOptionsFactory.fromSystemProperties("my.random.namespace")
We should use the full name of the option not split it with dots - dots
represent hierarchy not words separation.
interface FooOptions extends PipelineOptions {
getSomeOption()
getSomeOtherOption()
}
I think I can be +0.5 on this. We might also reserve a namespace like
"beam.TestPipeline.options" and use it for specification of test config.
Could be easier than embedding JSON in a property, in some cases. Easier to
override little pieces for sure.
Kenn
On Tue, Feb 13, 2018 at 9:29 AM, Romain Manni-Bucau <rm...@gmail.com>
wrote:
> makes sense, do we want beam.foo.bar -> --foo-bar conversion too?
>
>
> Romain Manni-Bucau
> @rmannibucau <https://twitter.com/rmannibucau> | Blog
> <https://rmannibucau.metawerx.net/> | Old Blog
> <http://rmannibucau.wordpress.com> | Github
> <https://github.com/rmannibucau> | LinkedIn
> <https://www.linkedin.com/in/rmannibucau> | Book
> <https://www.packtpub.com/application-development/java-ee-8-high-performance>
>
> 2018-02-13 18:19 GMT+01:00 Eugene Kirpichov <ki...@google.com>:
>
>> Neutral about this one: haven't seen a case where this was needed, but
>> don't see anything wrong with it either. One thing I'd recommend if you go
>> through with it, extract from system properties under "beam." rather than
>> all of them, to avoid clashes.
>>
>> On Tue, Feb 13, 2018, 7:53 AM Jean-Baptiste Onofré <jb...@nanthrax.net>
>> wrote:
>>
>>> Hi Romain,
>>>
>>> it sounds interesting to me, and doesn't break anything, so +1 from my
>>> side.
>>>
>>> Regards
>>> JB
>>>
>>> On 02/13/2018 03:42 PM, Romain Manni-Bucau wrote:
>>> > Hi guys,
>>> >
>>> > there are hacks in beam testing code to read the args from a system
>>> property but
>>> > I wonder if we shouldnt add a PipelineOptionsFactory.fromSys
>>> temProperties().
>>> >
>>> > It would iterate over the system properties and take all --xxx=foo as
>>> potential
>>> > argument it tries to bind.
>>> >
>>> > Rational behind that is to enable users to wrap the pipeline API but
>>> still
>>> > expose the pipeline options to end users for advanced cases.
>>> >
>>> > Any discussion on this kind of usages already? What do you think of
>>> this proposal?
>>> >
>>> > Side note: we can think about a fromEnv() too.
>>> >
>>> > Romain Manni-Bucau
>>> > @rmannibucau <https://twitter.com/rmannibucau> | Blog
>>> > <https://rmannibucau.metawerx.net/> | Old Blog
>>> > <http://rmannibucau.wordpress.com> | Github <
>>> https://github.com/rmannibucau> |
>>> > LinkedIn <https://www.linkedin.com/in/rmannibucau> | Book
>>> > <https://www.packtpub.com/application-development/java-ee-8-
>>> high-performance>
>>>
>>> --
>>> Jean-Baptiste Onofré
>>> jbonofre@apache.org
>>> http://blog.nanthrax.net
>>> Talend - http://www.talend.com
>>>
>>
>
Re: PipelineOptions fromSystemProps?
Posted by Romain Manni-Bucau <rm...@gmail.com>.
makes sense, do we want beam.foo.bar -> --foo-bar conversion too?
Romain Manni-Bucau
@rmannibucau <https://twitter.com/rmannibucau> | Blog
<https://rmannibucau.metawerx.net/> | Old Blog
<http://rmannibucau.wordpress.com> | Github <https://github.com/rmannibucau> |
LinkedIn <https://www.linkedin.com/in/rmannibucau> | Book
<https://www.packtpub.com/application-development/java-ee-8-high-performance>
2018-02-13 18:19 GMT+01:00 Eugene Kirpichov <ki...@google.com>:
> Neutral about this one: haven't seen a case where this was needed, but
> don't see anything wrong with it either. One thing I'd recommend if you go
> through with it, extract from system properties under "beam." rather than
> all of them, to avoid clashes.
>
> On Tue, Feb 13, 2018, 7:53 AM Jean-Baptiste Onofré <jb...@nanthrax.net>
> wrote:
>
>> Hi Romain,
>>
>> it sounds interesting to me, and doesn't break anything, so +1 from my
>> side.
>>
>> Regards
>> JB
>>
>> On 02/13/2018 03:42 PM, Romain Manni-Bucau wrote:
>> > Hi guys,
>> >
>> > there are hacks in beam testing code to read the args from a system
>> property but
>> > I wonder if we shouldnt add a PipelineOptionsFactory.
>> fromSystemProperties().
>> >
>> > It would iterate over the system properties and take all --xxx=foo as
>> potential
>> > argument it tries to bind.
>> >
>> > Rational behind that is to enable users to wrap the pipeline API but
>> still
>> > expose the pipeline options to end users for advanced cases.
>> >
>> > Any discussion on this kind of usages already? What do you think of
>> this proposal?
>> >
>> > Side note: we can think about a fromEnv() too.
>> >
>> > Romain Manni-Bucau
>> > @rmannibucau <https://twitter.com/rmannibucau> | Blog
>> > <https://rmannibucau.metawerx.net/> | Old Blog
>> > <http://rmannibucau.wordpress.com> | Github <https://github.com/
>> rmannibucau> |
>> > LinkedIn <https://www.linkedin.com/in/rmannibucau> | Book
>> > <https://www.packtpub.com/application-development/java-
>> ee-8-high-performance>
>>
>> --
>> Jean-Baptiste Onofré
>> jbonofre@apache.org
>> http://blog.nanthrax.net
>> Talend - http://www.talend.com
>>
>
Re: PipelineOptions fromSystemProps?
Posted by Eugene Kirpichov <ki...@google.com>.
Neutral about this one: haven't seen a case where this was needed, but
don't see anything wrong with it either. One thing I'd recommend if you go
through with it, extract from system properties under "beam." rather than
all of them, to avoid clashes.
On Tue, Feb 13, 2018, 7:53 AM Jean-Baptiste Onofré <jb...@nanthrax.net> wrote:
> Hi Romain,
>
> it sounds interesting to me, and doesn't break anything, so +1 from my
> side.
>
> Regards
> JB
>
> On 02/13/2018 03:42 PM, Romain Manni-Bucau wrote:
> > Hi guys,
> >
> > there are hacks in beam testing code to read the args from a system
> property but
> > I wonder if we shouldnt add a
> PipelineOptionsFactory.fromSystemProperties().
> >
> > It would iterate over the system properties and take all --xxx=foo as
> potential
> > argument it tries to bind.
> >
> > Rational behind that is to enable users to wrap the pipeline API but
> still
> > expose the pipeline options to end users for advanced cases.
> >
> > Any discussion on this kind of usages already? What do you think of this
> proposal?
> >
> > Side note: we can think about a fromEnv() too.
> >
> > Romain Manni-Bucau
> > @rmannibucau <https://twitter.com/rmannibucau> | Blog
> > <https://rmannibucau.metawerx.net/> | Old Blog
> > <http://rmannibucau.wordpress.com> | Github <
> https://github.com/rmannibucau> |
> > LinkedIn <https://www.linkedin.com/in/rmannibucau> | Book
> > <
> https://www.packtpub.com/application-development/java-ee-8-high-performance
> >
>
> --
> Jean-Baptiste Onofré
> jbonofre@apache.org
> http://blog.nanthrax.net
> Talend - http://www.talend.com
>
Re: PipelineOptions fromSystemProps?
Posted by Jean-Baptiste Onofré <jb...@nanthrax.net>.
Hi Romain,
it sounds interesting to me, and doesn't break anything, so +1 from my side.
Regards
JB
On 02/13/2018 03:42 PM, Romain Manni-Bucau wrote:
> Hi guys,
>
> there are hacks in beam testing code to read the args from a system property but
> I wonder if we shouldnt add a PipelineOptionsFactory.fromSystemProperties().
>
> It would iterate over the system properties and take all --xxx=foo as potential
> argument it tries to bind.
>
> Rational behind that is to enable users to wrap the pipeline API but still
> expose the pipeline options to end users for advanced cases.
>
> Any discussion on this kind of usages already? What do you think of this proposal?
>
> Side note: we can think about a fromEnv() too.
>
> Romain Manni-Bucau
> @rmannibucau <https://twitter.com/rmannibucau> | Blog
> <https://rmannibucau.metawerx.net/> | Old Blog
> <http://rmannibucau.wordpress.com> | Github <https://github.com/rmannibucau> |
> LinkedIn <https://www.linkedin.com/in/rmannibucau> | Book
> <https://www.packtpub.com/application-development/java-ee-8-high-performance>
--
Jean-Baptiste Onofré
jbonofre@apache.org
http://blog.nanthrax.net
Talend - http://www.talend.com