[FLINK-37511][rest] Use Jackson serialization in JobPlanInfo.Plan #26320
There are no files selected for viewing
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -509,7 +509,72 @@ | |
| "properties" : { | ||
| "plan" : { | ||
| "type" : "object", | ||
| "id" : "urn:jsonschema:org:apache:flink:runtime:rest:messages:JobPlanInfo:Plan", | ||
| "properties" : { | ||
| "jid" : { | ||
| "type" : "string" | ||
| }, | ||
| "name" : { | ||
| "type" : "string" | ||
| }, | ||
| "nodes" : { | ||
| "type" : "array", | ||
| "items" : { | ||
| "type" : "object", | ||
| "id" : "urn:jsonschema:org:apache:flink:runtime:rest:messages:JobPlanInfo:Plan:Node", | ||
| "properties" : { | ||
| "description" : { | ||
| "type" : "string" | ||
| }, | ||
| "id" : { | ||
| "type" : "string" | ||
| }, | ||
| "inputs" : { | ||
| "type" : "array", | ||
| "items" : { | ||
| "type" : "object", | ||
| "id" : "urn:jsonschema:org:apache:flink:runtime:rest:messages:JobPlanInfo:Plan:Node:Input", | ||
| "properties" : { | ||
| "caching" : { | ||
| "type" : "string" | ||
| }, | ||
| "exchange" : { | ||
| "type" : "string" | ||
| }, | ||
| "id" : { | ||
| "type" : "string" | ||
| }, | ||
| "local_strategy" : { | ||
| "type" : "string" | ||
| }, | ||
| "num" : { | ||
| "type" : "integer" | ||
| }, | ||
| "ship_strategy" : { | ||
| "type" : "string" | ||
| } | ||
| } | ||
| } | ||
| }, | ||
| "operator" : { | ||
| "type" : "string" | ||
| }, | ||
| "operator_strategy" : { | ||
| "type" : "string" | ||
| }, | ||
| "optimizer_properties" : { | ||
| "type" : "string" | ||
| }, | ||
| "parallelism" : { | ||
| "type" : "integer" | ||
| } | ||
| } | ||
| } | ||
| }, | ||
| "type" : { | ||
| "type" : "string" | ||
| } | ||
| } | ||
| } | ||
| } | ||
| }</code></pre> | ||
|
|
@@ -1126,7 +1191,72 @@ | |
| }, | ||
| "plan" : { | ||
| "type" : "object", | ||
| "id" : "urn:jsonschema:org:apache:flink:runtime:rest:messages:JobPlanInfo:Plan", | ||
| "properties" : { | ||
| "jid" : { | ||
| "type" : "string" | ||
| }, | ||
| "name" : { | ||
| "type" : "string" | ||
| }, | ||
| "nodes" : { | ||
| "type" : "array", | ||
| "items" : { | ||
| "type" : "object", | ||
| "id" : "urn:jsonschema:org:apache:flink:runtime:rest:messages:JobPlanInfo:Plan:Node", | ||
| "properties" : { | ||
| "description" : { | ||
| "type" : "string" | ||
| }, | ||
| "id" : { | ||
| "type" : "string" | ||
| }, | ||
| "inputs" : { | ||
| "type" : "array", | ||
| "items" : { | ||
| "type" : "object", | ||
| "id" : "urn:jsonschema:org:apache:flink:runtime:rest:messages:JobPlanInfo:Plan:Node:Input", | ||
| "properties" : { | ||
| "caching" : { | ||
| "type" : "string" | ||
| }, | ||
| "exchange" : { | ||
| "type" : "string" | ||
| }, | ||
| "id" : { | ||
| "type" : "string" | ||
| }, | ||
| "local_strategy" : { | ||
| "type" : "string" | ||
| }, | ||
| "num" : { | ||
| "type" : "integer" | ||
| }, | ||
| "ship_strategy" : { | ||
| "type" : "string" | ||
| } | ||
| } | ||
| } | ||
| }, | ||
| "operator" : { | ||
| "type" : "string" | ||
| }, | ||
| "operator_strategy" : { | ||
| "type" : "string" | ||
| }, | ||
| "optimizer_properties" : { | ||
| "type" : "string" | ||
| }, | ||
| "parallelism" : { | ||
| "type" : "integer" | ||
| } | ||
| } | ||
| } | ||
| }, | ||
| "type" : { | ||
| "type" : "string" | ||
| } | ||
| } | ||
| }, | ||
| "start-time" : { | ||
| "type" : "integer" | ||
|
|
@@ -1143,7 +1273,7 @@ | |
| }, | ||
| "stream-graph" : { | ||
| "type" : "object", | ||
| "id" : "urn:jsonschema:org:apache:flink:runtime:rest:messages:JobPlanInfo:RawJson" | ||
| }, | ||
| "timestamps" : { | ||
| "type" : "object", | ||
|
|
@@ -2879,7 +3009,72 @@ | |
| "properties" : { | ||
| "plan" : { | ||
| "type" : "object", | ||
| "id" : "urn:jsonschema:org:apache:flink:runtime:rest:messages:JobPlanInfo:Plan", | ||
|
Contributor
There was a problem hiding this comment. I see there are still several open questions(cc. @davidradl ):
Kind of, the RawJson didn't generate the proper docs and rest_api indeed looked unreadable with RawJson
Yes, indeed we used to serialize it manually but right now Jackson does it for us based on POJO class but in fact, the serialization remains the same(it's still Jackson) but it just happens in different time/place
It doesn't look like a big change since, as we discussed in (https://github.com/apache/flink/pull/26320/files#r2022598714), the result JSON hasn't changed. So the only change is the internal representation changed from String to Object and the documentation has more details now. @Efrat19 maybe you have something to add. |
||
| "properties" : { | ||
| "jid" : { | ||
| "type" : "string" | ||
| }, | ||
| "name" : { | ||
| "type" : "string" | ||
| }, | ||
| "nodes" : { | ||
| "type" : "array", | ||
| "items" : { | ||
| "type" : "object", | ||
| "id" : "urn:jsonschema:org:apache:flink:runtime:rest:messages:JobPlanInfo:Plan:Node", | ||
| "properties" : { | ||
| "description" : { | ||
| "type" : "string" | ||
| }, | ||
| "id" : { | ||
| "type" : "string" | ||
| }, | ||
| "inputs" : { | ||
| "type" : "array", | ||
| "items" : { | ||
| "type" : "object", | ||
| "id" : "urn:jsonschema:org:apache:flink:runtime:rest:messages:JobPlanInfo:Plan:Node:Input", | ||
| "properties" : { | ||
| "caching" : { | ||
| "type" : "string" | ||
| }, | ||
| "exchange" : { | ||
| "type" : "string" | ||
| }, | ||
| "id" : { | ||
| "type" : "string" | ||
| }, | ||
| "local_strategy" : { | ||
| "type" : "string" | ||
| }, | ||
| "num" : { | ||
| "type" : "integer" | ||
| }, | ||
| "ship_strategy" : { | ||
| "type" : "string" | ||
| } | ||
| } | ||
| } | ||
| }, | ||
| "operator" : { | ||
| "type" : "string" | ||
| }, | ||
| "operator_strategy" : { | ||
| "type" : "string" | ||
| }, | ||
| "optimizer_properties" : { | ||
| "type" : "string" | ||
| }, | ||
| "parallelism" : { | ||
| "type" : "integer" | ||
| } | ||
| } | ||
| } | ||
| }, | ||
| "type" : { | ||
| "type" : "string" | ||
| } | ||
| } | ||
| } | ||
| } | ||
| }</code></pre> | ||
|
|
||
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Oops, something went wrong.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Uh oh!
There was an error while loading. Please reload this page.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Have you been able to check if the result json looks the same? I mean, we should be sure that the old client can safely read the json that is created by this new version
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks. the returned response doesn't change (except null fields which are added)
I updated the description with example, wdyt?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yes, it looks good. Two new fields should not be a problem. It's important that the old fields remain the same, and they do.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@akalash @Efrat19 thanks for the explanations. From the discussion - this seems reasonable. I am curious as to what the 2 new null fields mean, do we gain anything from them? I am thinking we can now differentiate between a field that does not exist an one that exists but has a null value. Do we have tests that can drive each of these cases? Or is this not an important aspect to this - as they are effectively treated the same?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
For nulls, should we mark the fields with @JsonInclude(Include.NON_NULL) (I guess that should hide them if they're null)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks. Addressed in a followup PR #26488