Some concerns would be: * How does a user discover what options can be set and what values they take? * System properties are global so how would you prevent system properties from conflicting within Apache Beam and with other non Apache Beam libraries that may rely on system properties? * There are users who launch more then one pipeline from the same application, how would system properties work there?
Note that the system property usage is only for integration testing and not meant for users writing pipelines. On Thu, Jan 11, 2018 at 9:38 PM, Romain Manni-Bucau <rmannibu...@gmail.com> wrote: > Hmm, here is my thought: allowing main options settings is super important > and one of the most important user experience point. I d even say any IO > config shouldnt be an auto value but an option and through the transform > name you should be able to prefux the option name and override a particular > IO config through the command line - just to say how important it is and > avois hack by IO for that. > > The fact there is this global system property, json integration, and in > this PR another system property which is a bit weird for a user IMHO can > mean we nees to update the config design. If each option has an equivalent > system property then no need of all these hacks. > > Ex: tempLocation would support -DtempLocation, -Dbeam.tempLocation, > -D<options simplename|qualified name>.templocation (+ transform name prefix > if we add IO config one day) > > Then jackson is only here to parse the options....which is already done > manually since options use a custom serializer so it can be dropped, no? > > Is this reasonning wrong? > > Le 12 janv. 2018 00:11, "Lukasz Cwik" <lc...@google.com> a écrit : > >> Robert Bradshaw had the idea of migrating away from using main(String[] >> args) and just refactoring the code to test the WordCount PTransform >> allowing one to write a traditional JUnit test that didn't call >> main(String[] args). This would change what the contents of our examples >> are and make them more amenable to testing which could be good guidance for >> developers as well. >> >> On Thu, Jan 11, 2018 at 3:08 PM, Lukasz Cwik <lc...@google.com> wrote: >> >>> 1) TestPipeline#convertToArgs is meant to convert TestPipelineOptions >>> into a String[] containing --arg=value for integration tests that only have >>> a main(String[] arg) entry point like WordCount. There is this PR[1] that >>> is outstanding that is attempting to clean this up and simplify it so that >>> we aren't doing this haphazard conversion. >>> [1] https://github.com/apache/beam/pull/4346 >>> >>> I haven't yet commented on the PR but I was going to suggest that the >>> user add String[] getMainArgs to TestPipelineOptions and not do the >>> convertToArgs hackiness for ITs that use main(). >>> >>> On Thu, Jan 11, 2018 at 1:53 PM, Romain Manni-Bucau < >>> rmannibu...@gmail.com> wrote: >>> >>>> Some inputs - maybe comments? - would be appreciated on: >>>> >>>> 1. Why using json in https://github.com/apache/b >>>> eam/blob/master/sdks/java/core/src/main/java/org/apache/beam >>>> /sdk/testing/TestPipeline.java#L480 and not iterate over the options? >>>> 2. Also this is likely used to do a fromArgs and create another >>>> pipeline options later no? Is this api that useful here or does it need an >>>> options composition - fully object? >>>> >>>> Le 11 janv. 2018 19:09, "Lukasz Cwik" <lc...@google.com> a écrit : >>>> >>>>> Give links to the code segments that you want background on. >>>>> >>>>> On Wed, Jan 10, 2018 at 12:44 AM, Romain Manni-Bucau < >>>>> rmannibu...@gmail.com> wrote: >>>>> >>>>>> updated my junit 5 PR to show that: https://github.com/apach >>>>>> e/beam/pull/4360/files#diff-578d1770f8b47ebbc1e74a2c19de9a6aR28 >>>>>> >>>>>> It doesn't remove jackson yet but exposes a nicer user interface for >>>>>> the config. >>>>>> >>>>>> I'm not fully clear on all the jackson usage yet, there are some >>>>>> round trips (PO -> json -> PO) which are weird without more knowledge. >>>>>> >>>>>> >>>>>> 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> >>>>>> >>>>>> 2018-01-09 22:57 GMT+01:00 Lukasz Cwik <lc...@google.com>: >>>>>> >>>>>>> Removing large dependencies is great but I can only assume it will >>>>>>> be a lot of work so if you can get it to work in a backwards compatible >>>>>>> way >>>>>>> great. >>>>>>> >>>>>>> On Tue, Jan 9, 2018 at 1:52 PM, Romain Manni-Bucau < >>>>>>> rmannibu...@gmail.com> wrote: >>>>>>> >>>>>>>> It conflicts easily between libs and containers. Shade is not a >>>>>>>> good option too - see the thread on this topic :(. >>>>>>>> >>>>>>>> At the end i see using the cli solution closer to user - vs >>>>>>>> framework dev for json - and hurting less in terms of classpath so pby >>>>>>>> sthg >>>>>>>> to test no? >>>>>>>> >>>>>>>> Le 9 janv. 2018 22:47, "Lukasz Cwik" <lc...@google.com> a écrit : >>>>>>>> >>>>>>>>> Romain, how has Jackson been a classpath breaker? >>>>>>>>> >>>>>>>>> >>>>>>>>> On Tue, Jan 9, 2018 at 1:20 PM, Romain Manni-Bucau < >>>>>>>>> rmannibu...@gmail.com> wrote: >>>>>>>>> >>>>>>>>>> Hmm, beam already owns the cli parsing - that is what I meant - >>>>>>>>>> it only misses the arg delimiter (ie quoting) and adding it is easy >>>>>>>>>> no? >>>>>>>>>> >>>>>>>>>> Le 9 janv. 2018 21:19, "Robert Bradshaw" <rober...@google.com> a >>>>>>>>>> écrit : >>>>>>>>>> >>>>>>>>>>> On Tue, Jan 9, 2018 at 11:48 AM, Romain Manni-Bucau >>>>>>>>>>> <rmannibu...@gmail.com> wrote: >>>>>>>>>>> > >>>>>>>>>>> > Le 9 janv. 2018 19:50, "Robert Bradshaw" <rober...@google.com> >>>>>>>>>>> a écrit : >>>>>>>>>>> > >>>>>>>>>>> > From what I understand: >>>>>>>>>>> > >>>>>>>>>>> > 1) The command line argument values (both simple and more >>>>>>>>>>> complex) are >>>>>>>>>>> > all JSON-representable. >>>>>>>>>>> > >>>>>>>>>>> > And must be all CLI representable >>>>>>>>>>> >>>>>>>>>>> Sorry, I should have said "all pipeline options." In any case, >>>>>>>>>>> one can >>>>>>>>>>> always do JSON -> string and the resulting string is usable as a >>>>>>>>>>> command line argument. >>>>>>>>>>> >>>>>>>>>>> > 2) The command line is a mapping of keys to these values. >>>>>>>>>>> > >>>>>>>>>>> > Which makes your 1 not true since json supports more ;) >>>>>>>>>>> > >>>>>>>>>>> > As such, it seems quite natural to represent the whole set as >>>>>>>>>>> a single >>>>>>>>>>> > JSON map, rather than using a different, custom encoding for >>>>>>>>>>> the top >>>>>>>>>>> > level (whose custom escaping would have to be carried into the >>>>>>>>>>> inner >>>>>>>>>>> > JSON values). Note that JSON has the advantage that one never >>>>>>>>>>> needs to >>>>>>>>>>> > explain or define it, and parsers/serializers already exists >>>>>>>>>>> for all >>>>>>>>>>> > languages (e.g. if one has a driver script in another language >>>>>>>>>>> for >>>>>>>>>>> > launching a java pipeline, it's easy to communicate all the >>>>>>>>>>> args). >>>>>>>>>>> > >>>>>>>>>>> > Same reasonning applies to CLI AFAIK >>>>>>>>>>> >>>>>>>>>>> The spec of what a valid command line argument list is is >>>>>>>>>>> surprisingly >>>>>>>>>>> inconsistent across platforms, languages, and programs. And your >>>>>>>>>>> proposal seems to be getting into what the delimiter is, and how >>>>>>>>>>> to >>>>>>>>>>> escape it, and possibly then how to escape the escape character. >>>>>>>>>>> All >>>>>>>>>>> of this is sidestepped by pointing at an existing spec. >>>>>>>>>>> >>>>>>>>>>> > We can't get rid of Jackson in the core because of (1) so >>>>>>>>>>> there's >>>>>>>>>>> > little value in adding complexity to remove it from (2). The >>>>>>>>>>> fact that >>>>>>>>>>> > Java doesn't ship anything in its expansive standard library >>>>>>>>>>> for this >>>>>>>>>>> > is unfortuante, so we have to take a dependency on something. >>>>>>>>>>> > >>>>>>>>>>> > We actually can as shown before >>>>>>>>>>> >>>>>>>>>>> How, if JSON is integral to the parsing of the argument values >>>>>>>>>>> themselves? (Or is the argument that it's not, but each extender >>>>>>>>>>> of >>>>>>>>>>> PipelineOptions is responsible for choosing a JSON parser and >>>>>>>>>>> doing it >>>>>>>>>>> themselves.) >>>>>>>>>>> >>>>>>>>>>> > and jackson is not a simple negligible lib >>>>>>>>>>> > but a classpath breaker :( >>>>>>>>>>> >>>>>>>>>>> Perhaps there are better libraries we could use instead? Is the >>>>>>>>>>> ecosystem so bad that it encourages projects to roll their own of >>>>>>>>>>> everything rather than share code? >>>>>>>>>>> >>>>>>>>>>> > On Tue, Jan 9, 2018 at 10:00 AM, Lukasz Cwik <lc...@google.com> >>>>>>>>>>> wrote: >>>>>>>>>>> >> Ah, you don't want JSON within JSON. I see, if thats the case >>>>>>>>>>> just migrate >>>>>>>>>>> >> all of them to string tokenization and drop the Jackson usage >>>>>>>>>>> for string >>>>>>>>>>> >> -> >>>>>>>>>>> >> string[] conversion. >>>>>>>>>>> >> >>>>>>>>>>> >> On Mon, Jan 8, 2018 at 10:06 PM, Romain Manni-Bucau >>>>>>>>>>> >> <rmannibu...@gmail.com> >>>>>>>>>>> >> wrote: >>>>>>>>>>> >>> >>>>>>>>>>> >>> Lukasz, the use case is to znsure the config used can still >>>>>>>>>>> map the CLI >>>>>>>>>>> >>> and that options dont start to abuse json so it is exactly >>>>>>>>>>> the opposite >>>>>>>>>>> >>> of >>>>>>>>>>> >>> "we can be fancy" and is closer to "we can be robust". Also >>>>>>>>>>> the default >>>>>>>>>>> >>> should be easy and not json (i just want to set the runner, >>>>>>>>>>> --runner=xxx >>>>>>>>>>> >>> is >>>>>>>>>>> >>> easier than a json version). If the value doesnt start with >>>>>>>>>>> a [ we can >>>>>>>>>>> >>> use >>>>>>>>>>> >>> the tokenizer else jackson, wdyt? >>>>>>>>>>> >>> >>>>>>>>>>> >>> >>>>>>>>>>> >>> Le 8 janv. 2018 22:59, "Lukasz Cwik" <lc...@google.com> a >>>>>>>>>>> écrit : >>>>>>>>>>> >>> >>>>>>>>>>> >>> Now that I think about this more. I looked at some of the >>>>>>>>>>> examples in the >>>>>>>>>>> >>> pom.xml and they don't seem to be tricky to write in JSON. >>>>>>>>>>> >>> I also looked at the Jenkins job configurations >>>>>>>>>>> (specifically the >>>>>>>>>>> >>> performance tests) and they pass around maps which they >>>>>>>>>>> convert to the >>>>>>>>>>> >>> required format without needing a user to write it >>>>>>>>>>> themselves. >>>>>>>>>>> >>> Using gradle, we will be able to trivially to do the same >>>>>>>>>>> thing (convert >>>>>>>>>>> >>> maps to json without needing the person to write it by hand) >>>>>>>>>>> like groovy >>>>>>>>>>> >>> does. >>>>>>>>>>> >>> Since we are migrating away from Maven it doesn't seem >>>>>>>>>>> worthwhile to >>>>>>>>>>> >>> spend >>>>>>>>>>> >>> time on to make it easier to write the args in the Maven >>>>>>>>>>> poms. >>>>>>>>>>> >>> >>>>>>>>>>> >>> Is there another use case that is being missed? >>>>>>>>>>> >>> >>>>>>>>>>> >>> On Mon, Jan 8, 2018 at 1:38 PM, Romain Manni-Bucau >>>>>>>>>>> >>> <rmannibu...@gmail.com> >>>>>>>>>>> >>> wrote: >>>>>>>>>>> >>>> >>>>>>>>>>> >>>> Good point for \t and ,. >>>>>>>>>>> >>>> >>>>>>>>>>> >>>> Any objection to use jackson as a fallback for that purpose >>>>>>>>>>> - for >>>>>>>>>>> >>>> backwqrd compat only - and make it optional then? Will >>>>>>>>>>> create the ticket >>>>>>>>>>> >>>> if >>>>>>>>>>> >>>> not. >>>>>>>>>>> >>>> >>>>>>>>>>> >>>> Le 8 janv. 2018 20:32, "Robert Bradshaw" < >>>>>>>>>>> rober...@google.com> a écrit : >>>>>>>>>>> >>>>> >>>>>>>>>>> >>>>> Part of the motivation to use JSON for more complex >>>>>>>>>>> options was that >>>>>>>>>>> >>>>> it avoids having to define (and document, test, have users >>>>>>>>>>> learn, ...) >>>>>>>>>>> >>>>> yet another format for expressing lists, maps, etc. >>>>>>>>>>> >>>>> >>>>>>>>>>> >>>>> On Mon, Jan 8, 2018 at 11:19 AM, Lukasz Cwik < >>>>>>>>>>> lc...@google.com> wrote: >>>>>>>>>>> >>>>> > Ken, this is specifically about running integration >>>>>>>>>>> tests and not a >>>>>>>>>>> >>>>> > users >>>>>>>>>>> >>>>> > main(). >>>>>>>>>>> >>>>> > >>>>>>>>>>> >>>>> > Note, that PipelineOptions JSON format was used because >>>>>>>>>>> it was a >>>>>>>>>>> >>>>> > convenient >>>>>>>>>>> >>>>> > serialized form that is easy to explain to people what >>>>>>>>>>> is required. >>>>>>>>>>> >>>>> > Using a different string tokenizer and calling >>>>>>>>>>> >>>>> > PipelineOptionsFactory.fromArgs() with the parsed >>>>>>>>>>> strings seems like >>>>>>>>>>> >>>>> > it >>>>>>>>>>> >>>>> > would be better. >>>>>>>>>>> >>>>> > >>>>>>>>>>> >>>>> > These are the supported formats for fromArgs(): >>>>>>>>>>> >>>>> > * --project=MyProject (simple property, will set >>>>>>>>>>> the "project" >>>>>>>>>>> >>>>> > property >>>>>>>>>>> >>>>> > to "MyProject") >>>>>>>>>>> >>>>> > * --readOnly=true (for boolean properties, will set >>>>>>>>>>> the >>>>>>>>>>> >>>>> > "readOnly" >>>>>>>>>>> >>>>> > property to "true") >>>>>>>>>>> >>>>> > * --readOnly (shorthand for boolean properties, >>>>>>>>>>> will set the >>>>>>>>>>> >>>>> > "readOnly" >>>>>>>>>>> >>>>> > property to "true") >>>>>>>>>>> >>>>> > * --x=1 --x=2 --x=3 (list style simple property, >>>>>>>>>>> will set the >>>>>>>>>>> >>>>> > "x" >>>>>>>>>>> >>>>> > property to [1, 2, 3]) >>>>>>>>>>> >>>>> > * --x=1,2,3 (shorthand list style simple property, >>>>>>>>>>> will set the >>>>>>>>>>> >>>>> > "x" >>>>>>>>>>> >>>>> > property to [1, 2, 3]) >>>>>>>>>>> >>>>> > * --complexObject='{"key1":"value1",...} (JSON >>>>>>>>>>> format for all >>>>>>>>>>> >>>>> > other >>>>>>>>>>> >>>>> > complex types) >>>>>>>>>>> >>>>> > >>>>>>>>>>> >>>>> > Using a string tokenizer that minimizes the number of >>>>>>>>>>> required escape >>>>>>>>>>> >>>>> > characters would be good so we could use newline >>>>>>>>>>> characters as our >>>>>>>>>>> >>>>> > only >>>>>>>>>>> >>>>> > token. I would avoid ',\t ' as tokens since they are >>>>>>>>>>> more likely to >>>>>>>>>>> >>>>> > appear. >>>>>>>>>>> >>>>> > >>>>>>>>>>> >>>>> > On Mon, Jan 8, 2018 at 10:33 AM, Kenneth Knowles < >>>>>>>>>>> k...@google.com> >>>>>>>>>>> >>>>> > wrote: >>>>>>>>>>> >>>>> >> >>>>>>>>>>> >>>>> >> We do have a plain command line syntax, and whoever >>>>>>>>>>> writes the >>>>>>>>>>> >>>>> >> main(String[]) function is responsible for invoking the >>>>>>>>>>> parser. It >>>>>>>>>>> >>>>> >> isn't >>>>>>>>>>> >>>>> >> quite as nice as standard arg parse libraries, but it >>>>>>>>>>> isn't too bad. >>>>>>>>>>> >>>>> >> It >>>>>>>>>>> >>>>> >> would be great to improve, though. >>>>>>>>>>> >>>>> >> >>>>>>>>>>> >>>>> >> Jackson is for machine-to-machine communication or >>>>>>>>>>> other situations >>>>>>>>>>> >>>>> >> where >>>>>>>>>>> >>>>> >> command line parsing doesn't work so well. >>>>>>>>>>> >>>>> >> >>>>>>>>>>> >>>>> >> Are we using these some other way? >>>>>>>>>>> >>>>> >> >>>>>>>>>>> >>>>> >> Kenn >>>>>>>>>>> >>>>> >> >>>>>>>>>>> >>>>> >> On Sun, Jan 7, 2018 at 7:21 AM, Romain Manni-Bucau >>>>>>>>>>> >>>>> >> <rmannibu...@gmail.com> >>>>>>>>>>> >>>>> >> wrote: >>>>>>>>>>> >>>>> >>> >>>>>>>>>>> >>>>> >>> >>>>>>>>>>> >>>>> >>> >>>>>>>>>>> >>>>> >>> Le 7 janv. 2018 12:53, "Jean-Baptiste Onofré" < >>>>>>>>>>> j...@nanthrax.net> a >>>>>>>>>>> >>>>> >>> écrit : >>>>>>>>>>> >>>>> >>> >>>>>>>>>>> >>>>> >>> Hi Romain, >>>>>>>>>>> >>>>> >>> >>>>>>>>>>> >>>>> >>> I guess you are assuming that pipeline options are >>>>>>>>>>> flat command >>>>>>>>>>> >>>>> >>> line >>>>>>>>>>> >>>>> >>> like >>>>>>>>>>> >>>>> >>> argument, right ? >>>>>>>>>>> >>>>> >>> >>>>>>>>>>> >>>>> >>> Actually, theoretically, pipeline options can be >>>>>>>>>>> represented as >>>>>>>>>>> >>>>> >>> json, >>>>>>>>>>> >>>>> >>> that's why we use jackson. >>>>>>>>>>> >>>>> >>> The pipeline options can be serialized/deserialized as >>>>>>>>>>> json. >>>>>>>>>>> >>>>> >>> >>>>>>>>>>> >>>>> >>> >>>>>>>>>>> >>>>> >>> Yes but if users (or dev ;)) start to use that then it >>>>>>>>>>> is trivial >>>>>>>>>>> >>>>> >>> to >>>>>>>>>>> >>>>> >>> break the cli handling and fromArgs parsing or, if >>>>>>>>>>> not, break the >>>>>>>>>>> >>>>> >>> user >>>>>>>>>>> >>>>> >>> experience. So at the end it is a kind of "yes but >>>>>>>>>>> no", right? >>>>>>>>>>> >>>>> >>> >>>>>>>>>>> >>>>> >>> PS: already see some advanced users having a headache >>>>>>>>>>> trying to >>>>>>>>>>> >>>>> >>> pass >>>>>>>>>>> >>>>> >>> pipeline options in json so using the plain command >>>>>>>>>>> line syntax can >>>>>>>>>>> >>>>> >>> be more >>>>>>>>>>> >>>>> >>> friendly too. >>>>>>>>>>> >>>>> >>> >>>>>>>>>>> >>>>> >>> >>>>>>>>>>> >>>>> >>> The purpose is to remove the jackson dependencies ? >>>>>>>>>>> >>>>> >>> >>>>>>>>>>> >>>>> >>> >>>>>>>>>>> >>>>> >>> Later yes, seeing core dep tree I identify a lot of >>>>>>>>>>> dep which can >>>>>>>>>>> >>>>> >>> conflict in some env and are not really needed or >>>>>>>>>>> bring much being >>>>>>>>>>> >>>>> >>> in the >>>>>>>>>>> >>>>> >>> core - like avro as mentionned in another thread. Can >>>>>>>>>>> need a >>>>>>>>>>> >>>>> >>> sanitizing >>>>>>>>>>> >>>>> >>> round. Short term it was really a "why is it that >>>>>>>>>>> complicated" ;). >>>>>>>>>>> >>>>> >>> >>>>>>>>>>> >>>>> >>> >>>>>>>>>>> >>>>> >>> >>>>>>>>>>> >>>>> >>> Regards >>>>>>>>>>> >>>>> >>> JB >>>>>>>>>>> >>>>> >>> >>>>>>>>>>> >>>>> >>> >>>>>>>>>>> >>>>> >>> On 01/07/2018 11:38 AM, Romain Manni-Bucau wrote: >>>>>>>>>>> >>>>> >>>> >>>>>>>>>>> >>>>> >>>> Hi guys, >>>>>>>>>>> >>>>> >>>> >>>>>>>>>>> >>>>> >>>> not sure i fully get why jackson is used to parse >>>>>>>>>>> pipeline options >>>>>>>>>>> >>>>> >>>> in >>>>>>>>>>> >>>>> >>>> the testing integration >>>>>>>>>>> >>>>> >>>> >>>>>>>>>>> >>>>> >>>> why not using a token parser like [1] which allows to >>>>>>>>>>> map 1-1 with >>>>>>>>>>> >>>>> >>>> the >>>>>>>>>>> >>>>> >>>> user interface (command line) the options? >>>>>>>>>>> >>>>> >>>> >>>>>>>>>>> >>>>> >>>> [1] >>>>>>>>>>> >>>>> >>>> >>>>>>>>>>> >>>>> >>>> >>>>>>>>>>> >>>>> >>>> https://github.com/Talend/comp >>>>>>>>>>> onent-runtime/blob/master/component-server/src/main/java/org >>>>>>>>>>> /talend/sdk/component/server/lang/StringPropertiesTokenizer.java >>>>>>>>>>> >>>>> >>>> >>>>>>>>>>> >>>>> >>>> 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> >>>>>>>>>>> >>>>> >>> >>>>>>>>>>> >>>>> >>> >>>>>>>>>>> >>>>> >>> -- >>>>>>>>>>> >>>>> >>> Jean-Baptiste Onofré >>>>>>>>>>> >>>>> >>> jbono...@apache.org >>>>>>>>>>> >>>>> >>> http://blog.nanthrax.net >>>>>>>>>>> >>>>> >>> Talend - http://www.talend.com >>>>>>>>>>> >>>>> >>> >>>>>>>>>>> >>>>> >>> >>>>>>>>>>> >>>>> >> >>>>>>>>>>> >>>>> > >>>>>>>>>>> >>> >>>>>>>>>>> >>> >>>>>>>>>>> >>> >>>>>>>>>>> >> >>>>>>>>>>> > >>>>>>>>>>> > >>>>>>>>>>> >>>>>>>>>> >>>>>>>>> >>>>>>> >>>>>> >>>>> >>> >>
