You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@flink.apache.org by Gabor Somogyi <ga...@gmail.com> on 2023/12/06 09:52:49 UTC
[DISCUSS] REST API response parsing throws exception on new fields
Hi All,
Since the possible solution can have effect on all the REST response
deserialization I would like
ask for opinions.
*Problem statement:*
At the moment Flink is not ignoring unknown fields when parsing REST
responses. An example for such a class is JobDetailsInfo but this applies
to all others. It would be good to add this support to increase
compatibility.
The real life use-case is when the Flink k8s operator wants to handle 2
jobs with 2 different Flink versions where the newer version has added a
new field to any REST response. Such case the operator has basically 2
options:
* Use the old Flink version -> Such case exception comes because new field
comes but it's not expected
* Use the new Flink version -> Such case exception comes because new field
is not coming but expected
To hack around this issue it requires quite some ugly code parts in the
operator.
The mentioned issue is tracked here:
https://issues.apache.org/jira/browse/FLINK-33268
*Proposed solution:*
Ignore all unknown fields in case of REST response JSON deserialization.
Important to know that strict serialization would stay the same as-is.
Actual object mapper configuration can be found here:
https://github.com/apache/flink/blob/3060ccd49cc8d19634b431dbf0f09ac875d0d422/flink-runtime/src/main/java/org/apache/flink/runtime/rest/util/RestMapperUtils.java#L31-L38
Please share your opinion on this topic.
BR,
G
Re: [DISCUSS] REST API response parsing throws exception on new fields
Posted by Gabor Somogyi <ga...@gmail.com>.
Switching to the new thread...
G
On Thu, Dec 7, 2023 at 10:25 AM Gyula Fóra <gy...@gmail.com> wrote:
> You should start a new discussion thread according to the FLIP guidelines,
> so that it's identifiable that this is a FLIP discussion.
>
> Gyula
>
> On Thu, Dec 7, 2023 at 10:22 AM Gabor Somogyi <ga...@gmail.com>
> wrote:
>
> > Gyula, thanks for sharing your opinion.
> > I've created a small FLIP as asked:
> >
> >
> https://cwiki.apache.org/confluence/display/FLINK/FLIP-401%3A+REST+API+JSON+response+deserialization+unknown+field+tolerance
> >
> > Let's wait on voices until beginning of next week.
> >
> > G
> >
> >
> > On Wed, Dec 6, 2023 at 11:05 AM Gyula Fóra <gy...@gmail.com> wrote:
> >
> > > Thanks G, I think this is a reasonable proposal which will increase
> > > compatibility between different Flink clients and versions such as SQL
> > > Gateway, CLI, Flink operator etc.
> > >
> > > I don't really see any harm in ignoring unknown json fields globally,
> but
> > > this probably warrants a FLIP and a proper vote.
> > >
> > > Cheers,
> > > Gyula
> > >
> > > On Wed, Dec 6, 2023 at 10:53 AM Gabor Somogyi <
> gabor.g.somogyi@gmail.com
> > >
> > > wrote:
> > >
> > > > Hi All,
> > > >
> > > > Since the possible solution can have effect on all the REST response
> > > > deserialization I would like
> > > > ask for opinions.
> > > >
> > > > *Problem statement:*
> > > >
> > > > At the moment Flink is not ignoring unknown fields when parsing REST
> > > > responses. An example for such a class is JobDetailsInfo but this
> > applies
> > > > to all others. It would be good to add this support to increase
> > > > compatibility.
> > > >
> > > > The real life use-case is when the Flink k8s operator wants to
> handle 2
> > > > jobs with 2 different Flink versions where the newer version has
> added
> > a
> > > > new field to any REST response. Such case the operator has basically
> 2
> > > > options:
> > > > * Use the old Flink version -> Such case exception comes because new
> > > field
> > > > comes but it's not expected
> > > > * Use the new Flink version -> Such case exception comes because new
> > > field
> > > > is not coming but expected
> > > >
> > > > To hack around this issue it requires quite some ugly code parts in
> the
> > > > operator.
> > > >
> > > > The mentioned issue is tracked here:
> > > > https://issues.apache.org/jira/browse/FLINK-33268
> > > >
> > > > *Proposed solution:*
> > > >
> > > > Ignore all unknown fields in case of REST response JSON
> > deserialization.
> > > > Important to know that strict serialization would stay the same
> as-is.
> > > >
> > > > Actual object mapper configuration can be found here:
> > > >
> > > >
> > >
> >
> https://github.com/apache/flink/blob/3060ccd49cc8d19634b431dbf0f09ac875d0d422/flink-runtime/src/main/java/org/apache/flink/runtime/rest/util/RestMapperUtils.java#L31-L38
> > > >
> > > > Please share your opinion on this topic.
> > > >
> > > > BR,
> > > > G
> > > >
> > >
> >
>
Re: [DISCUSS] REST API response parsing throws exception on new fields
Posted by Gyula Fóra <gy...@gmail.com>.
You should start a new discussion thread according to the FLIP guidelines,
so that it's identifiable that this is a FLIP discussion.
Gyula
On Thu, Dec 7, 2023 at 10:22 AM Gabor Somogyi <ga...@gmail.com>
wrote:
> Gyula, thanks for sharing your opinion.
> I've created a small FLIP as asked:
>
> https://cwiki.apache.org/confluence/display/FLINK/FLIP-401%3A+REST+API+JSON+response+deserialization+unknown+field+tolerance
>
> Let's wait on voices until beginning of next week.
>
> G
>
>
> On Wed, Dec 6, 2023 at 11:05 AM Gyula Fóra <gy...@gmail.com> wrote:
>
> > Thanks G, I think this is a reasonable proposal which will increase
> > compatibility between different Flink clients and versions such as SQL
> > Gateway, CLI, Flink operator etc.
> >
> > I don't really see any harm in ignoring unknown json fields globally, but
> > this probably warrants a FLIP and a proper vote.
> >
> > Cheers,
> > Gyula
> >
> > On Wed, Dec 6, 2023 at 10:53 AM Gabor Somogyi <gabor.g.somogyi@gmail.com
> >
> > wrote:
> >
> > > Hi All,
> > >
> > > Since the possible solution can have effect on all the REST response
> > > deserialization I would like
> > > ask for opinions.
> > >
> > > *Problem statement:*
> > >
> > > At the moment Flink is not ignoring unknown fields when parsing REST
> > > responses. An example for such a class is JobDetailsInfo but this
> applies
> > > to all others. It would be good to add this support to increase
> > > compatibility.
> > >
> > > The real life use-case is when the Flink k8s operator wants to handle 2
> > > jobs with 2 different Flink versions where the newer version has added
> a
> > > new field to any REST response. Such case the operator has basically 2
> > > options:
> > > * Use the old Flink version -> Such case exception comes because new
> > field
> > > comes but it's not expected
> > > * Use the new Flink version -> Such case exception comes because new
> > field
> > > is not coming but expected
> > >
> > > To hack around this issue it requires quite some ugly code parts in the
> > > operator.
> > >
> > > The mentioned issue is tracked here:
> > > https://issues.apache.org/jira/browse/FLINK-33268
> > >
> > > *Proposed solution:*
> > >
> > > Ignore all unknown fields in case of REST response JSON
> deserialization.
> > > Important to know that strict serialization would stay the same as-is.
> > >
> > > Actual object mapper configuration can be found here:
> > >
> > >
> >
> https://github.com/apache/flink/blob/3060ccd49cc8d19634b431dbf0f09ac875d0d422/flink-runtime/src/main/java/org/apache/flink/runtime/rest/util/RestMapperUtils.java#L31-L38
> > >
> > > Please share your opinion on this topic.
> > >
> > > BR,
> > > G
> > >
> >
>
Re: [DISCUSS] REST API response parsing throws exception on new fields
Posted by Gabor Somogyi <ga...@gmail.com>.
Gyula, thanks for sharing your opinion.
I've created a small FLIP as asked:
https://cwiki.apache.org/confluence/display/FLINK/FLIP-401%3A+REST+API+JSON+response+deserialization+unknown+field+tolerance
Let's wait on voices until beginning of next week.
G
On Wed, Dec 6, 2023 at 11:05 AM Gyula Fóra <gy...@gmail.com> wrote:
> Thanks G, I think this is a reasonable proposal which will increase
> compatibility between different Flink clients and versions such as SQL
> Gateway, CLI, Flink operator etc.
>
> I don't really see any harm in ignoring unknown json fields globally, but
> this probably warrants a FLIP and a proper vote.
>
> Cheers,
> Gyula
>
> On Wed, Dec 6, 2023 at 10:53 AM Gabor Somogyi <ga...@gmail.com>
> wrote:
>
> > Hi All,
> >
> > Since the possible solution can have effect on all the REST response
> > deserialization I would like
> > ask for opinions.
> >
> > *Problem statement:*
> >
> > At the moment Flink is not ignoring unknown fields when parsing REST
> > responses. An example for such a class is JobDetailsInfo but this applies
> > to all others. It would be good to add this support to increase
> > compatibility.
> >
> > The real life use-case is when the Flink k8s operator wants to handle 2
> > jobs with 2 different Flink versions where the newer version has added a
> > new field to any REST response. Such case the operator has basically 2
> > options:
> > * Use the old Flink version -> Such case exception comes because new
> field
> > comes but it's not expected
> > * Use the new Flink version -> Such case exception comes because new
> field
> > is not coming but expected
> >
> > To hack around this issue it requires quite some ugly code parts in the
> > operator.
> >
> > The mentioned issue is tracked here:
> > https://issues.apache.org/jira/browse/FLINK-33268
> >
> > *Proposed solution:*
> >
> > Ignore all unknown fields in case of REST response JSON deserialization.
> > Important to know that strict serialization would stay the same as-is.
> >
> > Actual object mapper configuration can be found here:
> >
> >
> https://github.com/apache/flink/blob/3060ccd49cc8d19634b431dbf0f09ac875d0d422/flink-runtime/src/main/java/org/apache/flink/runtime/rest/util/RestMapperUtils.java#L31-L38
> >
> > Please share your opinion on this topic.
> >
> > BR,
> > G
> >
>
Re: [DISCUSS] REST API response parsing throws exception on new fields
Posted by Gyula Fóra <gy...@gmail.com>.
Thanks G, I think this is a reasonable proposal which will increase
compatibility between different Flink clients and versions such as SQL
Gateway, CLI, Flink operator etc.
I don't really see any harm in ignoring unknown json fields globally, but
this probably warrants a FLIP and a proper vote.
Cheers,
Gyula
On Wed, Dec 6, 2023 at 10:53 AM Gabor Somogyi <ga...@gmail.com>
wrote:
> Hi All,
>
> Since the possible solution can have effect on all the REST response
> deserialization I would like
> ask for opinions.
>
> *Problem statement:*
>
> At the moment Flink is not ignoring unknown fields when parsing REST
> responses. An example for such a class is JobDetailsInfo but this applies
> to all others. It would be good to add this support to increase
> compatibility.
>
> The real life use-case is when the Flink k8s operator wants to handle 2
> jobs with 2 different Flink versions where the newer version has added a
> new field to any REST response. Such case the operator has basically 2
> options:
> * Use the old Flink version -> Such case exception comes because new field
> comes but it's not expected
> * Use the new Flink version -> Such case exception comes because new field
> is not coming but expected
>
> To hack around this issue it requires quite some ugly code parts in the
> operator.
>
> The mentioned issue is tracked here:
> https://issues.apache.org/jira/browse/FLINK-33268
>
> *Proposed solution:*
>
> Ignore all unknown fields in case of REST response JSON deserialization.
> Important to know that strict serialization would stay the same as-is.
>
> Actual object mapper configuration can be found here:
>
> https://github.com/apache/flink/blob/3060ccd49cc8d19634b431dbf0f09ac875d0d422/flink-runtime/src/main/java/org/apache/flink/runtime/rest/util/RestMapperUtils.java#L31-L38
>
> Please share your opinion on this topic.
>
> BR,
> G
>