You are viewing a plain text version of this content. The canonical link for it is here.
Posted to issues@flink.apache.org by "zentol (via GitHub)" <gi...@apache.org> on 2023/04/05 13:54:53 UTC

[GitHub] [flink] zentol opened a new pull request, #22357: [FLINK-31735][docs] Document 'plan' field as object

zentol opened a new pull request, #22357:
URL: https://github.com/apache/flink/pull/22357

   Fixes an issue where `plan` field of the JobDetailsInfo was documented as a string, when in fact we write the json plan as a raw object.
   
   


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: issues-unsubscribe@flink.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


[GitHub] [flink] zentol commented on a diff in pull request #22357: [FLINK-31735][docs] Document 'plan' field as object

Posted by "zentol (via GitHub)" <gi...@apache.org>.
zentol commented on code in PR #22357:
URL: https://github.com/apache/flink/pull/22357#discussion_r1158561930


##########
flink-runtime/src/main/java/org/apache/flink/runtime/rest/messages/job/JobDetailsInfo.java:
##########
@@ -110,8 +113,8 @@ public class JobDetailsInfo implements ResponseBody {
     private final Map<ExecutionState, Integer> jobVerticesPerState;
 
     @JsonProperty(FIELD_NAME_JSON_PLAN)
-    @JsonRawValue
-    private final String jsonPlan;
+    @JsonSerialize(using = RawObjectSerializer.class)
+    private final Object jsonPlan;

Review Comment:
   Double-checking if we can re-use JobPlanInfo#RawJson



-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: issues-unsubscribe@flink.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


[GitHub] [flink] dmvk commented on pull request #22357: [FLINK-31735][docs] Document 'plan' field as object

Posted by "dmvk (via GitHub)" <gi...@apache.org>.
dmvk commented on PR #22357:
URL: https://github.com/apache/flink/pull/22357#issuecomment-1498914372

   nvm, it's correct, it just doesn't align with the HTML docs


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: issues-unsubscribe@flink.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


[GitHub] [flink] zentol commented on a diff in pull request #22357: [FLINK-31735][docs] Document 'plan' field as object

Posted by "zentol (via GitHub)" <gi...@apache.org>.
zentol commented on code in PR #22357:
URL: https://github.com/apache/flink/pull/22357#discussion_r1159667297


##########
docs/static/generated/rest_v1_dispatcher.yml:
##########
@@ -2304,7 +2304,7 @@ components:
           type: integer
           format: int64
         plan:
-          type: string

Review Comment:
   `any` isn't valie in the openapi spec



-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: issues-unsubscribe@flink.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


[GitHub] [flink] zentol commented on a diff in pull request #22357: [FLINK-31735][docs] Document 'plan' field as object

Posted by "zentol (via GitHub)" <gi...@apache.org>.
zentol commented on code in PR #22357:
URL: https://github.com/apache/flink/pull/22357#discussion_r1159680129


##########
docs/static/generated/rest_v1_dispatcher.yml:
##########
@@ -2304,7 +2304,7 @@ components:
           type: integer
           format: int64
         plan:
-          type: string

Review Comment:
   > is there any way we can document is as 'object' there?
   
   Yes; I've pushed a change for this.



-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: issues-unsubscribe@flink.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


[GitHub] [flink] zentol merged pull request #22357: [FLINK-31735][docs] Document 'plan' field as object

Posted by "zentol (via GitHub)" <gi...@apache.org>.
zentol merged PR #22357:
URL: https://github.com/apache/flink/pull/22357


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: issues-unsubscribe@flink.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


[GitHub] [flink] dmvk commented on a diff in pull request #22357: [FLINK-31735][docs] Document 'plan' field as object

Posted by "dmvk (via GitHub)" <gi...@apache.org>.
dmvk commented on code in PR #22357:
URL: https://github.com/apache/flink/pull/22357#discussion_r1159671577


##########
docs/static/generated/rest_v1_dispatcher.yml:
##########
@@ -2304,7 +2304,7 @@ components:
           type: integer
           format: int64
         plan:
-          type: string

Review Comment:
   ok, what causes confusion to me is that we document it as any in the htlm docs; is there any way we can document is as 'object' there?



-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: issues-unsubscribe@flink.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


[GitHub] [flink] zentol commented on a diff in pull request #22357: [FLINK-31735][docs] Document 'plan' field as object

Posted by "zentol (via GitHub)" <gi...@apache.org>.
zentol commented on code in PR #22357:
URL: https://github.com/apache/flink/pull/22357#discussion_r1158561930


##########
flink-runtime/src/main/java/org/apache/flink/runtime/rest/messages/job/JobDetailsInfo.java:
##########
@@ -110,8 +113,8 @@ public class JobDetailsInfo implements ResponseBody {
     private final Map<ExecutionState, Integer> jobVerticesPerState;
 
     @JsonProperty(FIELD_NAME_JSON_PLAN)
-    @JsonRawValue
-    private final String jsonPlan;
+    @JsonSerialize(using = RawObjectSerializer.class)
+    private final Object jsonPlan;

Review Comment:
   Currently checking if we can re-use JobPlanInfo#RawJson...



-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: issues-unsubscribe@flink.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


[GitHub] [flink] flinkbot commented on pull request #22357: [FLINK-31735][docs] Document 'plan' field as object

Posted by "flinkbot (via GitHub)" <gi...@apache.org>.
flinkbot commented on PR #22357:
URL: https://github.com/apache/flink/pull/22357#issuecomment-1497536552

   <!--
   Meta data
   {
     "version" : 1,
     "metaDataEntries" : [ {
       "hash" : "78f0b0ba194cf0edb59e868f635cf0ca9c0da149",
       "status" : "UNKNOWN",
       "url" : "TBD",
       "triggerID" : "78f0b0ba194cf0edb59e868f635cf0ca9c0da149",
       "triggerType" : "PUSH"
     } ]
   }-->
   ## CI report:
   
   * 78f0b0ba194cf0edb59e868f635cf0ca9c0da149 UNKNOWN
   
   <details>
   <summary>Bot commands</summary>
     The @flinkbot bot supports the following commands:
   
    - `@flinkbot run azure` re-run the last Azure build
   </details>


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: issues-unsubscribe@flink.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


[GitHub] [flink] zentol commented on pull request #22357: [FLINK-31735][docs] Document 'plan' field as object

Posted by "zentol (via GitHub)" <gi...@apache.org>.
zentol commented on PR #22357:
URL: https://github.com/apache/flink/pull/22357#issuecomment-1498926542

   > If I understand it correctly we should be able to pass anything in the rawjson, for example "string"
   
   Technically you are correct, but we only use RawJson for objects. Should this change the docs will be wrong again.


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: issues-unsubscribe@flink.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


[GitHub] [flink] zentol commented on a diff in pull request #22357: [FLINK-31735][docs] Document 'plan' field as object

Posted by "zentol (via GitHub)" <gi...@apache.org>.
zentol commented on code in PR #22357:
URL: https://github.com/apache/flink/pull/22357#discussion_r1158557164


##########
flink-runtime/src/main/java/org/apache/flink/runtime/rest/messages/job/JobDetailsInfo.java:
##########
@@ -110,8 +113,8 @@ public class JobDetailsInfo implements ResponseBody {
     private final Map<ExecutionState, Integer> jobVerticesPerState;
 
     @JsonProperty(FIELD_NAME_JSON_PLAN)
-    @JsonRawValue
-    private final String jsonPlan;
+    @JsonSerialize(using = RawObjectSerializer.class)
+    private final Object jsonPlan;

Review Comment:
   I couldn't find a better solution for the OpenAPI generator than to have this field be of type `Object`.



-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: issues-unsubscribe@flink.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


[GitHub] [flink] dmvk commented on a diff in pull request #22357: [FLINK-31735][docs] Document 'plan' field as object

Posted by "dmvk (via GitHub)" <gi...@apache.org>.
dmvk commented on code in PR #22357:
URL: https://github.com/apache/flink/pull/22357#discussion_r1159666526


##########
docs/static/generated/rest_v1_dispatcher.yml:
##########
@@ -2304,7 +2304,7 @@ components:
           type: integer
           format: int64
         plan:
-          type: string

Review Comment:
   🤔 do we also want to have "any" here instead?



-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: issues-unsubscribe@flink.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


[GitHub] [flink] zentol commented on a diff in pull request #22357: [FLINK-31735][docs] Document 'plan' field as object

Posted by "zentol (via GitHub)" <gi...@apache.org>.
zentol commented on code in PR #22357:
URL: https://github.com/apache/flink/pull/22357#discussion_r1159667297


##########
docs/static/generated/rest_v1_dispatcher.yml:
##########
@@ -2304,7 +2304,7 @@ components:
           type: integer
           format: int64
         plan:
-          type: string

Review Comment:
   `any` isn't a valid type in the openapi spec



-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: issues-unsubscribe@flink.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org