Hi everyone,
thanks for the livid discussion, it's great to see so many opinions and
ideas!
The point about underscores is a very valid point where the current FLIP,
if we were to stick with it, would have to be improved. I was going to say
that we should exclude that from the discussion about the merits of
different overall solutions, but I am afraid that this makes the "how to
name EVs" question even more convoluted, and that in turn directly impacts
the usefulness of the FLIP as a whole which is about a more convenient way
of configuring Flink; names which are too cryptic will not achieve that. So
in this regard I am in agreement with Chesnay.
After all these considerations, speaking from the Kubernetes context, it
seems to me that using the dynamic properties works best (I can use config
key names as-is) and requires no change, so I'm actually just leaning
towards that. However, the Kubernetes context is, I guess, not the only one
to consider.
Best regards
Ingo
On Tue, Jan 26, 2021 at 3:48 PM Chesnay Schepler <ches...@apache.org> wrote:
> Mind you that we could of course solve these character issues by first
> nailing down which characters we allow in keys (presumably: [a-z0-9-.]).
>
> On 1/26/2021 3:45 PM, Chesnay Schepler wrote:
> > Here's an example: My option key is custom.my_backend_option. With the
> > current design, the corresponding env variable would be
> > CUSTOM_MY_BACKEND_OPTION, which would be converted into
> > custom.my.backend.option .
> >
> > It is true that users could still parse the original system property
> > as a fall-back, but it seems to partially invalidate the goal and
> > introduce the potential for surprises and inconsistent behavior.
> >
> > What would happen if the option were already defined in the
> > flink-conf.yaml, but overwritten with the env variable? Users would
> > have to check every time they access a configuration whether the
> > system property was also set and resolve things manually. Naturally
> > things might also conflict with whatever prioritization we come up with.
> >
> > Now you might say that this is only necessary if the option contains
> > special characters, but then we're setting users up for a surprise
> > should they ever rename an existing option to contain an underscore.
> >
> > As for newlines, I wouldn't expect newline characters to appear within
> > DYNAMIC_VARIABLE, but I guess it would follow the same behavior as if
> > you would declare them on the command-line?
> >
> > One more downside I see is that from a users perspective I'd always
> > have to do this conversion manually. You can't just copy stuff from
> > the documentation (unless we duplicate every single mention), nor can
> > you easily switch between environment variables and dynamic
> > properties/flink-conf.yaml . For the use-cases that people seems to be
> > concerned about (where you have lots of variables) I would think that
> > this is a deal-breaker.
> >
> > On 1/26/2021 2:59 PM, Khachatryan Roman wrote:
> >> @Chesnay
> >> could you explain how underscores in user-defined properties would be
> >> affected with transformation like STATE_BACKEND -> state.backend?
> >> IIUC, this transformation happens in Flink and doesn't alter ENV
> >> vars, so
> >> the user app can still parse the original configuration.
> >> OTH, I'm a bit concerned whether the newline should be escaped by the
> >> user
> >> in DYNAMIC_VARIABLES.
> >>
> >> @Ingo Bürk <i...@ververica.com>
> >> I feel a bit lost in the discussion) Maybe we can put an intermediate
> >> summary of pros and cons of different approaches into the FLIP?
> >>
> >> And for completeness, we could combine DYNAMIC_VARIABLES approach with
> >> passing individual variables.
> >>
> >>
> >> Regards,
> >> Roman
> >>
> >>
> >> On Tue, Jan 26, 2021 at 12:54 PM Chesnay Schepler <ches...@apache.org>
> >> wrote:
> >>
> >>> I think we have to assume that some user has a custom config option
> >>> that
> >>> uses underscores.
> >>>
> >>> That said, we can probably assume that no one uses other special
> >>> characters like question marks and such, which are indeed allowed by
> >>> the
> >>> YAML spec.
> >>>
> >>> These kind of questions are precisely why I prefer the
> >>> DYNAMIC_VARIABLES
> >>> approach; you don't even have to worry about this stuff.
> >>> The only question we'd have to answer is whether manually defined
> >>> properties should take precedent or not.
> >>>
> >>> @Uce I can see how it could be cumbersome to modify, but at the same
> >>> time you can implement whatever other approach you want on top of it:
> >>>
> >>> // this is a /conceptual /example for an optional setting
> >>> DYNAMIC_VARIABLES="${REST_PORT_SETTING}"
> >>> if _someCondition_:
> >>> export REST_PORT_SETTING="-Drest.port=1234"
> >>>
> >>> // this is a /conceptual /example for a configurable setting
> >>> DYNAMIC_VARIABLES="-Drest.port=${MY_FANCY_VARIABLE:-8200}"
> >>> if _someCondition_:
> >>> export MY_FANCY_VARIABLE="1234"
> >>>
> >>> Additionally, this makes it quite easy to audit stuff, since we can
> >>> just
> >>> eagerly log what DYNAMIC_VARIABLES is set to.
> >>>
> >>> On 1/26/2021 12:48 PM, Xintong Song wrote:
> >>>> @Ufuk,
> >>>> I also don't find any existing options with underscores in their keys.
> >>>> However, I do not find any explicit rules forbid using them either.
> >>>> I'm
> >>> not
> >>>> saying this should block the FLIP. Just it would be nice to beware of
> >>> this
> >>>> issue, and maybe ensure the assumption with test cases if we finally
> >>> decide
> >>>> to go with these mapping rules.
> >>>>
> >>>> Thank you~
> >>>>
> >>>> Xintong Song
> >>>>
> >>>>
> >>>>
> >>>> On Tue, Jan 26, 2021 at 7:27 PM Ufuk Celebi <u...@apache.org> wrote:
> >>>>
> >>>>> @Xingtong: The assumption for the mapping was that we only have
> >>>>> dots and
> >>>>> hyphens in the keys. Do you have an example for a key which include
> >>>>> underscores? If underscores are common for keys (I couldn't find any
> >>>>> existing options that use it), it would certainly be a blocker for
> >>>>> the
> >>>>> discussed approach.
> >>>>>
> >>>>> On Tue, Jan 26, 2021, at 11:46 AM, Xintong Song wrote:
> >>>>>> - The naming conversions proposed in the FLIP seems not
> >>>>>> bijective
> >>> to
> >>>>> me.
> >>>>>> There could be problems if the configuration key contains
> >>> underscores.
> >>>>>> - a_b -> FLINK_CONFIG_a_b
> >>>>>> - FLINK_CONFIG_a_b -> a.b
> >>>
> >>>
> >
>
>
