Hi Yang, yeah, this is essentially the solution we also use in Ververica Platform (except that we use ${…}). I can't really add anything other than the previously stated reasons for why we decided to reject this, but of course I'm also open to this solution still. I don't think Flink necessarily needs to maintain a flink-conf.yaml with all environment variables; I would assume users generally would want to override only some properties, not most of them, and for a config like that basically any EV not set would end up causing an error.
Regards Ingo On Wed, Jan 27, 2021 at 4:42 AM Yang Wang <danrtsey...@gmail.com> wrote: > Hi all, sorry to butt in. > > I am little curious about why do we have to do the overwritten via > environment variables after > loading the configuration from file. Could we support to do the > substitution while loading the > "flink-conf.yaml" file? > > For example, I have the following config options in my flink-conf.yaml. > fs.oss.accessKeyId: $(FS_OSS_ACCESS_KEY_ID) > fs.oss.accessKeySecret: $(FS_OSS_ACCESS_KEY_SECRET) > > I expect these environment variables could be substituted when loading the > configuration file. It is > very similar to use "*envsubst < /tmp/flink-conf.yaml > > /tmp/flink-conf-1.yaml*". > > I know this is a rejected alternative. But I think some reasons could not > stand on. > * We could use $(FS_OSS_ACCESS_KEY_ID) instead of ${FS_OSS_ACCESS_KEY_ID} > for the environment definition > to avoid escape issues. I think the Kubernetes has the same behavior[1]. > Maybe many users are already familiar with it. > * Users do not need to know the conversion between flink config option and > environment name. Because they could use > any name they want. > * It is true that we need to maintain a flink configuration file > which simply maps all keys to some environment variables. But > for Yarn and K8s deployment(both standalone on K8s and native), we already > have such a file, which is shipped from client > or mounted from a ConfigMap. > > > @Ingo This solution could perfectly work with Kubernetes deployment and is > easier to use. We could use a ConfigMap for > storing the flink-conf.yaml, and using secrets to exposed as environment > variables for the authentication informations. > > > [1]. > > https://kubernetes.io/docs/tasks/inject-data-application/define-environment-variable-container/#using-environment-variables-inside-of-your-config > > > Best, > Yang > > Chesnay Schepler <ches...@apache.org> 于2021年1月27日周三 上午8:03写道: > > > In the end DYNAMIC_PROPERTIES behaves exactly like env.java.opts; > > meaning that the existing rules set by the JVM apply. > > > > Here's an example: export DYNAMIC_PROPERTIES='-Drest.port=1234 > > -Dother.option="iContainAn=Sign"' > > > > This would then be appended as is to the /java/ command. > > ( > > Conceptually at least; shells are annoying when it comes to > > quotes/whitespace; good old http://mywiki.wooledge.org/BashFAQ/050. > > Something like java ... $(eval echo ${DYNAMIC_PROPERTIES} handles > > quotes nicely, but no spaces in values. > > > > We should really move more logic from the scripts into the > > BashJavaUtils... > > ) > > > > On 1/26/2021 11:17 PM, Khachatryan Roman 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 . > > > 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 > > >>>>>>> > > >>>> > > > > >