+1 to Option 3
I'd rather have each SDK have a single point of well defined complexity to do
something general, than have to make tiny but simple changes. Less toil and
maintenance in the long run per SDK.
Similarly I don't have time to make it happen right now.
On Tue, Nov 13, 2018, 9:22 AM Thomas Weise <t...@apache.org
<mailto:t...@apache.org>> wrote:
Discovering options from the job server would be the only way to perform
full validation (and provide upfront help to the user).
The runner cannot perform full validation, since it is not aware of the user
and SDK options (that it has to forward to the SDK worker).
Special runner options flag to forward unknown options also wouldn't fully
solve the problem (besides being subject to change in the future). Let's say
runner understands --fancy-int-option and the user repeats that option
multiple times. Not knowing the type of option, the SDK will pass it as a
list and the runner will fail.
Replicating SDK options is a workaround for known runners but it really goes
against the idea of portability (making assumptions outside the API
contract). We already have runners implemented outside of Beam and hope for
the ecosystem to grow. What we do for options should also work for those
runners.
I'm with Luke here that options discovery provides the best user experience
and can address the other issues. Even the scenario of multiple intermediate
runners could be addressed by forwarding the unparsed options with the
discovery call. I don't see SDK implementation complexity as a significant
drawback so far.
Thomas
On Mon, Nov 12, 2018 at 2:30 PM Lukasz Cwik <lc...@google.com
<mailto:lc...@google.com>> wrote:
On Mon, Nov 12, 2018 at 9:38 AM Maximilian Michels <m...@apache.org
<mailto:m...@apache.org>> wrote:
Thank you Robert and Lukasz for your points.
> Note that I believe that we will want to have multiple URLs to
support cross language pipelines since we will want to be able to
ask other SDK languages/versions for their "list" of supported
PipelineOptions.
Why is that? The Runner itself is the source of truth for its
options.
Because other languages (or even different versions of the same
language) may have their own options. For example, the Go SDK talks to a
Java service which applies a SQL transform and returns the expanded form
(this may require knowledge of some options like credentials for the
filesystem, ...) and then talks to a Python service that performs
another transform expansion. Finally the pipeline containing Go, Java
and Python transforms is submitted to a runner and it performs its own
internal replacements/expansions related to executing the pipeline.
Everything else is SDK-related and should be validated there.
I imagined the process to go like this:
a) Parse options to find JobServer URL
a) Retrieve options from JobServer
c) Parse all options
...continue as always...
An option is just represented by a name and a type. There is nothing
more to it, at least as of now. So it should be possible to parse
them
in the SDK without much further work.
Nevertheless, I agree with your sentiment, Robert. The
"runner_option"
flag would prevent additional complexity. I still don't prefer it
because it's not nice from an end user perspective. If we were to
implement it, I would definitely go for the "option promotion" which
you
mentioned.
I hadn't thought about delegating runners, although the
PortableRunner
is basically a delegating Runner. If that was an important feature,
I
suppose the "runner_option" would be the preferred way.
All in all, since there doesn't seem to be an excitement to
implement
JobServer option retrieval and we will need the help of all SDK
developers, "runner_option" seems to be the more likely path.
I would say its a lack of time for people to improve this story over
others but it can be revisited at some point in the future and I agree
that using --runner_option as an interim provides value.
-Max
On 08.11.18 21:50, Lukasz Cwik wrote:
> The purpose of the spec would be to provide the names, type and
> descriptions of the options. We don't need anything beyond the
JSON
> types (string, number, bool, object, list) because the only
ambiguity we
> get is how do we parse command line string into the JSON type
(and that
> ambiguity is actually only between string and non-string since
all the
> other JSON types are unambiguous).
>
> Also, I believe the flow would be
> 1) Parse options
> a) Find the URL from args specified and/or additional methods
on
> PipelineOptions that exposes a programmatic way to set the URL
during
> parsing.
> b) Query URL for option specs
> c) Parse the remainder of the options
> 2) Construct pipeline
> 3) Choose runner
> 4) Submit job to runner
>
> Note that I believe that we will want to have multiple URLs to
support
> cross language pipelines since we will want to be able to ask
other SDK
> languages/versions for their "list" of supported PipelineOptions.
>
> On Thu, Nov 8, 2018 at 11:51 AM Robert Bradshaw
<rober...@google.com <mailto:rober...@google.com>
> <mailto:rober...@google.com <mailto:rober...@google.com>>> wrote:
>
> There's two questions here:
>
> (A) What do we do in the short term?
>
> I think adding every runner option to every SDK is not
sustainable
> (n*m work, assuming every SDK knows about every runner), and
having a
> patchwork of options that were added as one-offs to SDKs is
not
> desirable either. Furthermore, it seems difficult to parse
unknown
> options as if they were valid options, so my preference here
would be
> to just use a special runner_option flag. (One could also
pass a set
> of unparsed/unvalidated runner options to the runner, even if
they're
> not distinguished for the user, and runners (or any
intermediates)
> could run a "promote" operation that promotes any of these
unknowns
> that they recognize to real options before further
processing. The
> parsing would be done as repeated-string, and not be
intermingled with
> the actually validated options. This is essential a variant
of
> option 1.)
>
> (B) What do do in the long term? While the JobServer approach
sounds
> nice, I think it introduces a lot of complexity (we have too
much of
> that already) and still doesn't completely solve the
problem. In
> particular, it changes the flow from
>
> 1. Parse options
> 2. Construct pipeline
> 3. Choose runner
> 4. Submit job to runner
>
> to
>
> 1. Parse options
> 2. Construct pipeline
> 3. Choose runner
> 4a. Query runner for option specs
> 4b. Re-parse options
> 4c. Submit job to runner
>
> In particular, doing 4b in the SDK rather than just let the
runner
> itself do the validation as part of (4) doesn't save much and
forces
> us to come up with a (probably incomplete) spec as to how to
define
> options, their types, and their validations. It also means
that a
> delegating runner must choose and interact with its
downstream
> runner(s) synchronously, else we haven't actually solved the
issue.
>
> For these reasons, I don't think we even want to go with the
JobServer
> approach in the long term, which has bearing on (A).
>
> - Robert
>
>
> On Wed, Nov 7, 2018 at 8:50 PM Maximilian Michels
<m...@apache.org <mailto:m...@apache.org>
> <mailto:m...@apache.org <mailto:m...@apache.org>>> wrote:
> >
> > +1
> >
> > If the preferred approach is to eventually have the
JobServer
> serve the
> > options, then the best intermediate solution is to
replicate common
> > options in the SDKs.
> >
> > If we went down the "--runner_option" path, we would end
up with
> > multiple ways of specifying the same options. We would
eventually
> have
> > to deprecate "runner options" once we have the JobServer
> approach. I'd
> > like to avoid that.
> >
> > For the upcoming release we can revert the changes again
and add the
> > most common missing options to the SDKs. Then hopefully we
should
> have
> > fetching implemented for the release after.
> >
> > Do you think that is feasible?
> >
> > Thanks,
> > Max
> >
> > On 30.10.18 23:00, Lukasz Cwik wrote:
> > > I still like #3 the most, just can't devote the time to
get it
> done.
> > >
> > > Instead of going with a fully implemented #3, we could
hardcode
> the a
> > > subset of options and types within each SDK until the
job server is
> > > ready to provide this information and then migrate to
the
> "full" list.
> > > This would be an easy path for SDKs to take on. They
could
> "know" of a
> > > few well known options, and if they want to support all
> options, they
> > > implement the integration with the job server.
> > >
> > > On Fri, Oct 26, 2018 at 9:19 AM Maximilian Michels
> <m...@apache.org <mailto:m...@apache.org>
<mailto:m...@apache.org <mailto:m...@apache.org>>
> > > <mailto:m...@apache.org <mailto:m...@apache.org>
<mailto:m...@apache.org <mailto:m...@apache.org>>>> wrote:
> > >
> > > > I would prefer we don't introduce a (quirky) way
of passing
> > > unknown options that forces users to type JSON into
the
> command line
> > > (or similar acrobatics)
> > > Same here, the JSON approach seems technically nice
but too
> bulky
> > > for users.
> > >
> > > > To someone wanting to run a pipeline, all
options are
> equally
> > > important, whether they are application specific,
SDK
> specific or
> > > runner specific.
> > >
> > > I'm also reluctant to force users to use
`--runner_option=`
> because the
> > > division into "Runner" options and other options
seems
> rather arbitrary
> > > to users. Most built-in options are also
Runner-related.
> > >
> > > > It should be possible to *optionally*
qualify/scope (to
> cover
> > > cases where there is ambiguity), but otherwise I
prefer the
> format
> > > we currently have.
> > >
> > > Yes, namespacing is a problem. What happens if the
user
> defines a
> > > custom
> > > PipelineOption which clashes with one of the builtin
ones?
> If both are
> > >
> > > set, which one is actually applied?
> > >
> > >
> > > Note that PipelineOptions so far has been treating name
> equality to mean
> > > option equality and the Java implementation has a bunch
of
> strict checks
> > > to make sure that default values aren't used for
duplicate
> definitions,
> > > they have the same type, etc...
> > > With 1), you fail the job if the runner can't understand
your
> option
> > > because its not represented the same way. User then
needs to fix-up
> > > their declaration of the option name.
> > > With 2), there are no name conflicts, the SDK will need
to
> validate that
> > > the option isn't set in both formats and error out if it
is before
> > > pipeline submission time.
> > > With 3), you can prefetch all the options and error out
to the user
> > > during argument parsing time.
> > >
> > >
> > >
> > > Here is a summary of the possible paths going
forward:
> > >
> > >
> > > 1) Validate PipelineOptions at Runner side
> > > ==========================================
> > >
> > > The main issue raised here was that we want to move
away
> from parsing
> > > arguments which look like options without validating
them.
> An easy fix
> > > would be to actually validate them on the Runner
side. This
> could be
> > > done by changing the deserialization code of
> PipelineOptions which so
> > > far ignores unknown JSON options.
> > >
> > > See: PipelineOptionsTranslation.fromProto(Struct
protoOptions)
> > >
> > > Actually, this wouldn't work for user-defined
> PipelineOptions because
> > > they might not be known to the Runner (if they are
defined
> in Python).
> > >
> > >
> > > 2) Introduce a Runner-Option Flag
> > > =================================
> > >
> > > In this approach we would try to add as many
pipeline
> options for a
> > > Runner to the SDK, but allow additional Runner
options to
> be passed
> > > using the `--runner-option=key=val` flag. The
Runner, like
> in 1), would
> > > have to ensure validation. I think this has been the
most
> favored
> > > way so
> > > far. Going forward, that means that
`--parallelism=4` and
> > > `--runner-option=parallelism=4` will have the same
effect
> for the Flink
> > > Runner.
> > >
> > >
> > > 3) Implement Fetching of Options from JobServer
> > > ===============================================
> > >
> > > The options are retrieved from the JobServer before
> submitting the
> > > pipeline. I think this would be ideal but, as
mentioned
> before, it
> > > increases the complexity for implementing new SDKs
and
> might overall
> > > just not be worth the effort.
> > >
> > >
> > > What do you think? I'd implement 2) for the next
release,
> unless there
> > > are advocates for a different approach.
> > >
> > > Cheers,
> > > Max
>
