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 <rmannibu...@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 <kirpic...@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é <j...@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é
>>> jbono...@apache.org
>>> http://blog.nanthrax.net
>>> Talend - http://www.talend.com
>>>
>>
>

Reply via email to