I do think it is the responsibility of the CLI to own this; we definitely shouldn't rewrite PipelineOptions validation entirely, or programmatic uses will have needless noise or bad messages. I think that when someone calls fromArgs() they have sufficiently indicated that they are a CLI code path and we can do the right thing.
Separate from that, it might be handy to expose such a utility for users that build their command line options via some other mechanism. On Mon, Jun 26, 2017 at 3:07 PM, Lukasz Cwik <[email protected]> wrote: > In this case, we specifically asked in code to validate the options: > https://github.com/apache/beam/blob/1ea1de4aa9d32e3c5a596ccd7d84af > f1cc2a7428/examples/java/src/main/java/org/apache/beam/ > examples/WordCount.java#L173 > > It seems like we could know whether PipelineOptions validation is happening > from the PipelineOptionsFactory.fromArgs() and/or have a special CLI > option > like --validate (similar to --help) that would provide names based upon > arguments. > > In general, the PipelineOptionsValidator / .withValidation() method should > provide information about what method has not been set when not used from a > CLI. > > Or we make sure both are listed to not have this dichotomy based upon how > PipelineOptions are used. > > On Mon, Jun 26, 2017 at 10:10 AM, Kenneth Knowles <[email protected]> > wrote: > > > We should decouple CLI parsing validation from programmatic > PipelineOptions > > validation. > > > > On Mon, Jun 26, 2017 at 9:56 AM, Lukasz Cwik <[email protected]> > > wrote: > > > > > This change assumes that all users are CLI users (compared to the > > existing > > > code which assumed that all users were programmatic users). Should we > > word > > > the message so its useful for both CLI and users who set the options > > > programmatically? > > > > > > On Mon, Jun 26, 2017 at 6:00 AM, Jean-Baptiste Onofré <[email protected] > > > > > wrote: > > > > > > > Thanks Manu ! > > > > > > > > I will review it. > > > > > > > > Regards > > > > JB > > > > > > > > > > > > On 06/26/2017 02:59 PM, Manu Zhang wrote: > > > > > > > >> Thanks Kenn and JB, I just filed > > > >> https://issues.apache.org/jira/browse/BEAM-2514 and PR > > > >> https://github.com/apache/beam/pull/3438 > > > >> Please help to review. > > > >> > > > >> Thanks, > > > >> Manu > > > >> > > > >> On Mon, Jun 26, 2017 at 12:20 PM Jean-Baptiste Onofré < > > [email protected]> > > > >> wrote: > > > >> > > > >> Hi Manu, > > > >>> > > > >>> Agree, it makes sense. > > > >>> > > > >>> Regards > > > >>> JB > > > >>> > > > >>> On 06/25/2017 12:57 PM, Manu Zhang wrote: > > > >>> > > > >>>> Hi all, > > > >>>> > > > >>>> Currently, if a required option is missing for a Beam pipeline, > the > > > >>>> error > > > >>>> message is like > > > >>>> > > > >>>> > > > >>>>> Exception in thread "main" java.lang.IllegalArgumentException: > > > Missing > > > >>>>> required value for [public abstract java.lang.String > > > >>>>> org.apache.beam.examples.WordCount$WordCountOptions.getOutput(), > > > "Path > > > >>>>> > > > >>>> of > > > >>> > > > >>>> the file to write to"]. > > > >>>>> > > > >>>> > > > >>>> > > > >>>> This is quite long but doesn't give any hint to users about the > > > required > > > >>>> option. Instead, I'm thinking about something like > > > >>>> > > > >>>> Exception in thread "main" java.lang.IllegalArgumentException: > > > Missing > > > >>>> > > > >>>>> required option [--output, "Path of the file to write to"]. > > > >>>>> > > > >>>> > > > >>>> > > > >>>> It can be achieved by adding a method that *returns the option > from > > a > > > >>>> method* to ProxyInvocationHandler > > > >>>> < > > > >>>> > > > >>> https://github.com/apache/beam/blob/master/sdks/java/core/ > > > >>> src/main/java/org/apache/beam/sdk/options/ > > ProxyInvocationHandler.java > > > >>> > > > >>>> > > > >>>> class. > > > >>>> > > > >>>> public String getOption(Method method) { > > > >>>> return gettersToPropertyNames.get(method.getName()); > > > >>>> } > > > >>>> > > > >>>> > > > >>>> This may look general for ProxyInvocationHandler but you get the > > idea. > > > >>>> > > > >>>> WDYT? > > > >>>> > > > >>>> Thanks, > > > >>>> Manu > > > >>>> > > > >>>> > > > >>> -- > > > >>> Jean-Baptiste Onofré > > > >>> [email protected] > > > >>> http://blog.nanthrax.net > > > >>> Talend - http://www.talend.com > > > >>> > > > >>> > > > >> > > > > -- > > > > Jean-Baptiste Onofré > > > > [email protected] > > > > http://blog.nanthrax.net > > > > Talend - http://www.talend.com > > > > > > > > > >
