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