> 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 .

I think we don't have to translate CUSTOM_MY_BACKEND_OPTION back. Instead,
we should use the key from the ConfigOption.
I'm assuming that not  every ENV VAR will end up in the Configuration -
only those for which a matching ConfigOptions is found.

I'm also fine with a single ENV VAR (DYNAMIC_PROPERTIES). It's already a
big improvement.
In the future, we can consider adding smth like ConfigOption.withEnvVar for
some (most popular) options.

However, escaping is still not clear to me: how would kv-pairs be
separated? What if such a separator is in the value itself? What if '=' is
in the value?
Or am I missing something?

Regards,
Roman


On Tue, Jan 26, 2021 at 6:41 PM Till Rohrmann <trohrm...@apache.org> wrote:

> Thinking a bit more about the DYNAMIC_PROPERTIES, I have to admit that I
> like the fact that it works around the problem of encoding the key names
> and that it is more powerful wrt to bulk changes. Also the fact that one
> can copy past configuration snippets is quite useful. Given these aspects
> and that we wouldn't exclude any mentioned configuration scenarios, I would
> also be ok following this approach given that we support it for all Flink
> processes.
>
> Cheers,
> Till
>
> On Tue, Jan 26, 2021 at 5:10 PM Ingo Bürk <i...@ververica.com> wrote:
>
>> 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
>> > >>>
>> > >>>
>> > >
>> >
>> >
>>
>

Reply via email to