Hi Chesnay, I think the affected options are a bit more than we thought. I created a sub-task [1] to add checks to guarantee the non-deprecated options not conflicting with standard YAML for newly added options in the future.
[1] https://issues.apache.org/jira/browse/FLINK-29410 Best Yun Tang ________________________________ From: Chesnay Schepler <ches...@apache.org> Sent: Friday, September 23, 2022 21:02 To: dev@flink.apache.org <dev@flink.apache.org>; Matthias Pohl <matthias.p...@aiven.io.INVALID> Subject: Re: [DISCUSS][FLINK-29372] Add a suffix to keys that violate YAML spec The PR contains a test now. This is a list of affected options and their new suffix: * env.java.opts{.all] * kubernetes.pod-template-file[.default] * execution.checkpointing.unaligned[.enabled] * high-availability[.type] * restart-strategy[.type] * cleanup-strategy[.type] * state.backend[.type] * kubernetes.jobmanager.cpu[.amount] * kubernetes.taskmanager.cpu[.amount] * kubernetes.container.image[.ref] * metrics.scope.jm.job -> metrics.scope.jm-job * metrics.scope.tm.job -> metrics.scope.tm-job On 22/09/2022 14:15, Matthias Pohl wrote: > +1 That sounds like a good idea. Does it make sense to add a validation > step to prevent new configuration parameters from running into the same > issue (with whitelisting the existing ones)? > > Matthias > > On Thu, Sep 22, 2022 at 6:35 AM Yun Tang <myas...@live.com> wrote: > >> +1 >> >> This is the prerequisite to help us to introduce a standard YAML parser, >> which has been discussed in different tickets [1] [2]. >> >> >> [1] https://issues.apache.org/jira/browse/FLINK-23620 >> [2] https://issues.apache.org/jira/browse/FLINK-29366 >> >> Best >> Yun Tang >> ________________________________ >> From: Yang Wang <danrtsey...@gmail.com> >> Sent: Thursday, September 22, 2022 11:00 >> To: dev@flink.apache.org <dev@flink.apache.org> >> Subject: Re: [DISCUSS][FLINK-29372] Add a suffix to keys that violate YAML >> spec >> >> This will make it possible to replace the current rough implementation[1] >> with a common yaml parser. >> And then we could avoid some unexpected behaviors[2]. >> >> +1 >> >> [1]. >> >> https://github.com/apache/flink/blob/master/flink-core/src/main/java/org/apache/flink/configuration/GlobalConfiguration.java#L179 >> [2]. https://issues.apache.org/jira/browse/FLINK-15358 >> >> Best, >> Yang >> >> Konstantin Knauf <kna...@apache.org> 于2022年9月22日周四 04:26写道： >> >>> Make sense to me. It is moving us in the right direction and makes it >>> possible to drop these keys with Flink 2.0 if that ever happens :) >>> >>> Am Mi., 21. Sept. 2022 um 16:06 Uhr schrieb Chesnay Schepler < >>> ches...@apache.org>: >>> >>>> Hi, >>>> >>>> we have a small number of options in Flink whose key is a prefix of >>>> other keys. >>>> >>>> This violates the YAML spec; when you view the options as a tree only >>>> leaf nodes may have properties. >>>> >>>> While this is a minor issue from our side I think this can be quite >>>> annoying for users, since it means you can't read or validate a Flink >>>> config with standard yaml tools. >>>> >>>> I'd like to add a suffix to those keys to resolve this particular >>>> problem, while still supporting the previous keys (as deprecated). >>>> >>>> AFAICT there aren't any risks here, >>>> except if users have a search&replace step for one of these options in >>>> the default config of the Flink distribution; >>>> however this seems unsafe in any case since the contents of the default >>>> config may change. >>>> >>>> This would also bring us a step closer to our goal of using a compliant >>>> YAML parser. >>>> >>> >>> -- >>> https://twitter.com/snntrable >>> https://github.com/knaufk >>>
