Re: [DISCUSS][FLINK-29372] Add a suffix to keys that violate YAML spec
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
Sent: Friday, September 23, 2022 21:02
To: dev@flink.apache.org ; Matthias Pohl
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 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
Sent: Thursday, September 22, 2022 11:00
To: 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 于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 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
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
Sent: Friday, September 23, 2022 21:02
To: dev@flink.apache.org ; Matthias Pohl
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 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
>> Sent: Thursday, September 22, 2022 11:00
>> To: 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 于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 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
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 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
Sent: Thursday, September 22, 2022 11:00
To: 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 于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 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
+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 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 > Sent: Thursday, September 22, 2022 11:00 > To: 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 于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 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
+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 Sent: Thursday, September 22, 2022 11:00 To: 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 于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 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
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 于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 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
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 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
[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 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.
