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