You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@flink.apache.org by Chesnay Schepler <ch...@apache.org> on 2022/09/21 14:06:26 UTC
[DISCUSS][FLINK-29372] Add a suffix to keys that violate YAML spec
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.
Re: [DISCUSS][FLINK-29372] Add a suffix to keys that violate YAML spec
Posted by Chesnay Schepler <ch...@apache.org>.
As I said, the PR already contains a test to safeguard against that.
On 26/09/2022 08:25, Yun Tang wrote:
> 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 <ch...@apache.org>
> Sent: Friday, September 23, 2022 21:02
> To: dev@flink.apache.org <de...@flink.apache.org>; Matthias Pohl <ma...@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 <my...@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 <da...@gmail.com>
>>> Sent: Thursday, September 22, 2022 11:00
>>> To: dev@flink.apache.org <de...@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 <kn...@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 <
>>>> chesnay@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
>>>>
>
Re: [DISCUSS][FLINK-29372] Add a suffix to keys that violate YAML spec
Posted by Yun Tang <my...@live.com>.
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 <ch...@apache.org>
Sent: Friday, September 23, 2022 21:02
To: dev@flink.apache.org <de...@flink.apache.org>; Matthias Pohl <ma...@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 <my...@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 <da...@gmail.com>
>> Sent: Thursday, September 22, 2022 11:00
>> To: dev@flink.apache.org <de...@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 <kn...@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 <
>>> chesnay@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
>>>
Re: [DISCUSS][FLINK-29372] Add a suffix to keys that violate YAML spec
Posted by Chesnay Schepler <ch...@apache.org>.
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 <my...@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 <da...@gmail.com>
>> Sent: Thursday, September 22, 2022 11:00
>> To: dev@flink.apache.org <de...@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 <kn...@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 <
>>> chesnay@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
>>>
Re: [DISCUSS][FLINK-29372] Add a suffix to keys that violate YAML spec
Posted by Matthias Pohl <ma...@aiven.io.INVALID>.
+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 <my...@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 <da...@gmail.com>
> Sent: Thursday, September 22, 2022 11:00
> To: dev@flink.apache.org <de...@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 <kn...@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 <
> > chesnay@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
> >
>
Re: [DISCUSS][FLINK-29372] Add a suffix to keys that violate YAML spec
Posted by Yun Tang <my...@live.com>.
+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 <da...@gmail.com>
Sent: Thursday, September 22, 2022 11:00
To: dev@flink.apache.org <de...@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 <kn...@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 <
> chesnay@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
>
Re: [DISCUSS][FLINK-29372] Add a suffix to keys that violate YAML spec
Posted by Yang Wang <da...@gmail.com>.
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 <kn...@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 <
> chesnay@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
>
Re: [DISCUSS][FLINK-29372] Add a suffix to keys that violate YAML spec
Posted by Konstantin Knauf <kn...@apache.org>.
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 <
chesnay@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