The problem I see with $FLINK_PROPERTIES is that this is only supported by Flink's Docker entrypoint.sh. In fact this variable was introduced as a temporary bridge to allow Docker users to change Flink's configuration dynamically. If we had had a proper way of configuring Flink processes via env variables, then we wouldn't have introduced it.
Cheers, Till On Mon, Jan 25, 2021 at 5:01 PM Ingo Bürk <i...@ververica.com> wrote: > Hi Chesnay, > > thanks for your input! After some thought I think your proposal makes a > lot of sense, i.e. if we have one single $FLINK_DYNAMIC_PROPERTIES > environment variable, in a Kubernetes environment we could do something like > > ``` > env: > - name: S3_KEY > valueFrom: # get from secret > - name: FLINK_DYNAMIC_PROPERTIES > value: -Ds3.access-key=$(S3_KEY) > ``` > > This, much like the substitution approach we rejected previously, requires > "intermediate" variables. However, they're all defined in the same scope > here, and this solution certainly has the noteworthy benefit that no "magic > syntax" (s3.access-key → FLINK_CONFIG_S3_ACCESS__KEY) is needed, which I > believe makes for a pretty good user experience here. I think personally I > prefer this approach over the approach we currently have in the FLIP, but > I'm definitely happy to hear thoughts from the others on this approach! > Especially maybe also regarding the test randomization point raised > by Khachatryan earlier in the discussion. > > > Regards > Ingo > > On Fri, Jan 22, 2021 at 5:18 PM Chesnay Schepler <ches...@apache.org> > wrote: > >> The FLIP seems to disregard the existence of dynamic properties, and I'm >> wondering why that is the case. >> Don't they fulfill similar roles, in that they allow config options to >> be set on the command-line? >> >> What use-case do they currently not support? >> I assume it's something along the lines of setting some environment >> variable for containers, but at least for those based on our docker >> images we already have a mechanism to support that. >> >> In any case, wouldn't a single DYNAMIC_PROPERTIES variable suffice that >> is automatically evaluated by all scripts? >> Why go through all the hassle with the environment variable names? >> >> On 1/22/2021 3:53 PM, Till Rohrmann wrote: >> > The updated design LGTM as well. Nice work Ingo! >> > >> > Cheers, >> > Till >> > >> > On Fri, Jan 22, 2021 at 3:33 PM Ingo Bürk <i...@ververica.com> wrote: >> > >> >> Thanks, Ufuk. I think that makes sense, so I moved it from a footnote >> to an >> >> addition to prevent that in the future as well. >> >> >> >> Ingo >> >> >> >> On Fri, Jan 22, 2021 at 3:10 PM Ufuk Celebi <u...@apache.org> wrote: >> >> >> >>> LGTM. Let's see what the others think... >> >>> >> >>> On Thu, Jan 21, 2021, at 11:37 AM, Ingo Bürk wrote: >> >>>> Regarding env.java.opts, what special handling is needed there? >> AFAICT >> >>> only >> >>>> the rejected alternative of substituting values would've had an >> effect >> >> on >> >>>> this. >> >>> Makes sense 👍 >> >>> >> >>> From the FLIP: >> >>>> This mapping is not strictly bijective, but cases with consecutive >> >>> periods or dashes in the key name are not considered here and should >> not >> >>> (reasonably) be allowed. >> >>> >> >>> We could actually enforce such restrictions in the implementation of >> >>> ConfigOption, avoiding any surprises down the line. >> >>> >> >>> – Ufuk >> >>> >> >>
