You are viewing a plain text version of this content. The canonical link for it is here.
Posted to issues@flink.apache.org by "nikhilsu (via GitHub)" <gi...@apache.org> on 2026/04/08 07:56:42 UTC
[PR] [hotfix][docs] Add SlotSharingGroupId to OpenAPI spec generator ID schema overrides [flink]
nikhilsu opened a new pull request, #27903:
URL: https://github.com/apache/flink/pull/27903
## What is the purpose of the change
The `SlotSharingGroupId` type is serialized as a hex string by `SlotSharingGroupIDSerializer` (added in [FLINK-20090](https://issues.apache.org/jira/browse/FLINK-20090)), but the OpenAPI spec generator produces an incorrect object schema with `bytes`, `lowerPart`, `upperPart` fields because `overrideIdSchemas()` does not include `SlotSharingGroupId`.
This causes API clients generated from the OpenAPI spec to fail when decoding `getJobDetails` responses containing
`slotSharingGroupId` fields.
Note: This change does not break backward compatibility issues as the previous object schema for `SlotSharingGroupId` never matched the actual wire format, so any client generated from the spec would have failed to decode this field. The corrected `type: string` schema matches what the REST API actually returns.
#### Affected endpoints:
- `GET /jobs/:jobid` (`JobDetailsInfo.JobVertexDetailsInfo.slotSharingGroupId`)
## Brief change log
- Added `SlotSharingGroupId` to `OpenApiSpecGenerator.overrideIdSchemas()` so it is treated as `type: string` with pattern `[0-9a-f]{32}`, matching `JobID`, `JobVertexID`, `ResourceID`, `IntermediateDataSetID`, and `TriggerId`.
## Verifying this change
This change is already covered by existing tests. The generated OpenAPI spec (`rest_v1_dispatcher.yml`) will now
correctly define `SlotSharingGroupId` as a string type instead of an object.
## Does this pull request potentially affect one of the following parts:
- Dependencies (does it add or upgrade a dependency): no
- The public API, i.e., is any changed class annotated with `@Public(Evolving)`: no
- The serializers: no
- The runtime per-record code paths (performance sensitive): no
- Anything that affects deployment or recovery: JobManager (and its components), Checkpointing, Kubernetes/Yarn, ZooKeeper: no
- The S3 file system connector: no
## Documentation
- Does this pull request introduce a new feature? no
- If yes, how is the feature documented? not applicable
--
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
Re: [PR] [hotfix][docs] Add SlotSharingGroupId to OpenAPI spec generator ID schema overrides [flink]
Posted by "flinkbot (via GitHub)" <gi...@apache.org>.
flinkbot commented on PR #27903:
URL: https://github.com/apache/flink/pull/27903#issuecomment-4204742916
<!--
Meta data
{
"version" : 1,
"metaDataEntries" : [ {
"hash" : "662f7f418c3f86d6cf4a85edce1f659ff6d11c0d",
"status" : "UNKNOWN",
"url" : "TBD",
"triggerID" : "662f7f418c3f86d6cf4a85edce1f659ff6d11c0d",
"triggerType" : "PUSH"
} ]
}-->
## CI report:
* 662f7f418c3f86d6cf4a85edce1f659ff6d11c0d 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
Re: [PR] [FLINK-39422] [REST][docs] Fix OpenAPI spec for `SlotSharingGroupId`, `MetricCollectionResponseBody`, and `SerializedThrowable` custom serializers [flink]
Posted by "nikhilsu (via GitHub)" <gi...@apache.org>.
nikhilsu commented on code in PR #27903:
URL: https://github.com/apache/flink/pull/27903#discussion_r3083851357
##########
flink-docs/src/main/java/org/apache/flink/docs/rest/OpenApiSpecGenerator.java:
##########
@@ -279,15 +282,33 @@ private static void overrideIdSchemas(final OpenAPI openApi) {
.addSchemas(JobVertexID.class.getSimpleName(), idSchema)
.addSchemas(IntermediateDataSetID.class.getSimpleName(), idSchema)
.addSchemas(TriggerId.class.getSimpleName(), idSchema)
- .addSchemas(ResourceID.class.getSimpleName(), idSchema);
+ .addSchemas(ResourceID.class.getSimpleName(), idSchema)
+ .addSchemas(SlotSharingGroupId.class.getSimpleName(), idSchema);
+ }
+
+ /**
+ * Overrides the schema for {@link MetricCollectionResponseBody} which uses a custom Jackson
+ * serializer that writes the metrics collection as a raw JSON array, not as an object with a
+ * "metrics" field.
+ */
+ private static void overrideMetricCollectionSchema(final OpenAPI openApi) {
Review Comment:
Btw, @RocMarshal now that we've updated `MetricCollectionResponseBody` to mirror the structure of `ConfigurationInfo`, for the sake of consistency we should also do the same in case of:
[AggregatedMetricsResponseBody.java](https://github.com/apache/flink/blob/release-2.2/flink-runtime/src/main/java/org/apache/flink/runtime/rest/messages/job/metrics/AggregatedMetricsResponseBody.java#L55)
--
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
Re: [PR] [FLINK-39422] [REST][docs] Fix OpenAPI spec for `SlotSharingGroupId`, `MetricCollectionResponseBody`, and `SerializedThrowable` custom serializers [flink]
Posted by "RocMarshal (via GitHub)" <gi...@apache.org>.
RocMarshal commented on code in PR #27903:
URL: https://github.com/apache/flink/pull/27903#discussion_r3068205221
##########
flink-docs/src/main/java/org/apache/flink/docs/rest/OpenApiSpecGenerator.java:
##########
@@ -279,15 +282,33 @@ private static void overrideIdSchemas(final OpenAPI openApi) {
.addSchemas(JobVertexID.class.getSimpleName(), idSchema)
.addSchemas(IntermediateDataSetID.class.getSimpleName(), idSchema)
.addSchemas(TriggerId.class.getSimpleName(), idSchema)
- .addSchemas(ResourceID.class.getSimpleName(), idSchema);
+ .addSchemas(ResourceID.class.getSimpleName(), idSchema)
+ .addSchemas(SlotSharingGroupId.class.getSimpleName(), idSchema);
+ }
+
+ /**
+ * Overrides the schema for {@link MetricCollectionResponseBody} which uses a custom Jackson
+ * serializer that writes the metrics collection as a raw JSON array, not as an object with a
+ * "metrics" field.
+ */
+ private static void overrideMetricCollectionSchema(final OpenAPI openApi) {
Review Comment:
Hi, @nikhilsu
In my limited reading,Perhaps implementing the MetricCollectionResponseBodyclass in the style of
org.apache.flink.runtime.rest.messages.ConfigurationInfowould be a good choice.
Main considerations are as follows:
- We wouldn’t need to modify the OpenApiSpecGeneratorclass when introducing similar new classes in the future
- There is already a precedent with the implementation of ConfigurationInfo, and we wouldn’t need to explicitly introduce serialization and deserialization processors
- In the REST API’s HTML and YML documentation, the definition of the return type would be relatively more detailed. For example, an object type would be refined to something like `Array<Metric>`
--
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
Re: [PR] [FLINK-39422] [REST][docs] Fix OpenAPI spec for `SlotSharingGroupId`, `MetricCollectionResponseBody`, and `SerializedThrowable` custom serializers [flink]
Posted by "RocMarshal (via GitHub)" <gi...@apache.org>.
RocMarshal commented on code in PR #27903:
URL: https://github.com/apache/flink/pull/27903#discussion_r3084025983
##########
flink-docs/src/main/java/org/apache/flink/docs/rest/OpenApiSpecGenerator.java:
##########
@@ -279,15 +282,33 @@ private static void overrideIdSchemas(final OpenAPI openApi) {
.addSchemas(JobVertexID.class.getSimpleName(), idSchema)
.addSchemas(IntermediateDataSetID.class.getSimpleName(), idSchema)
.addSchemas(TriggerId.class.getSimpleName(), idSchema)
- .addSchemas(ResourceID.class.getSimpleName(), idSchema);
+ .addSchemas(ResourceID.class.getSimpleName(), idSchema)
+ .addSchemas(SlotSharingGroupId.class.getSimpleName(), idSchema);
+ }
+
+ /**
+ * Overrides the schema for {@link MetricCollectionResponseBody} which uses a custom Jackson
+ * serializer that writes the metrics collection as a raw JSON array, not as an object with a
+ * "metrics" field.
+ */
+ private static void overrideMetricCollectionSchema(final OpenAPI openApi) {
Review Comment:
Hi， @nikhilsu
If the current implementation in this `[AggregatedMetricsResponseBody.java](https://github.com/apache/flink/blob/release-2.2/flink-runtime/src/main/java/org/apache/flink/runtime/rest/messages/job/metrics/AggregatedMetricsResponseBody.java#L55)` file does not cause the previous issues when generating the corresponding documentation, then I would prefer to keep it as is.
WDYTA?
--
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
Re: [PR] [FLINK-39422] [REST][docs] Fix OpenAPI spec for `SlotSharingGroupId`, `MetricCollectionResponseBody`, and `SerializedThrowable` custom serializers [flink]
Posted by "RocMarshal (via GitHub)" <gi...@apache.org>.
RocMarshal commented on PR #27903:
URL: https://github.com/apache/flink/pull/27903#issuecomment-4286015541
Hi, @snuyanzin Could you help evaluate the current changes and their impact on the stability of the REST API?
Any input is appreciated !
--
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
Re: [PR] [FLINK-39422] [REST][docs] Fix OpenAPI spec for `SlotSharingGroupId`, `MetricCollectionResponseBody`, and `SerializedThrowable` custom serializers [flink]
Posted by "RocMarshal (via GitHub)" <gi...@apache.org>.
RocMarshal commented on code in PR #27903:
URL: https://github.com/apache/flink/pull/27903#discussion_r3084025983
##########
flink-docs/src/main/java/org/apache/flink/docs/rest/OpenApiSpecGenerator.java:
##########
@@ -279,15 +282,33 @@ private static void overrideIdSchemas(final OpenAPI openApi) {
.addSchemas(JobVertexID.class.getSimpleName(), idSchema)
.addSchemas(IntermediateDataSetID.class.getSimpleName(), idSchema)
.addSchemas(TriggerId.class.getSimpleName(), idSchema)
- .addSchemas(ResourceID.class.getSimpleName(), idSchema);
+ .addSchemas(ResourceID.class.getSimpleName(), idSchema)
+ .addSchemas(SlotSharingGroupId.class.getSimpleName(), idSchema);
+ }
+
+ /**
+ * Overrides the schema for {@link MetricCollectionResponseBody} which uses a custom Jackson
+ * serializer that writes the metrics collection as a raw JSON array, not as an object with a
+ * "metrics" field.
+ */
+ private static void overrideMetricCollectionSchema(final OpenAPI openApi) {
Review Comment:
Hi， @nikhilsu
If the current implementation in this `AggregatedMetricsResponseBody.java` file does not cause the previous issues when generating the corresponding documentation, then I would prefer to keep it as is.
WDYTA?
--
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
Re: [PR] [FLINK-39422] [REST][docs] Fix OpenAPI spec for `SlotSharingGroupId`, `MetricCollectionResponseBody`, and `SerializedThrowable` custom serializers [flink]
Posted by "RocMarshal (via GitHub)" <gi...@apache.org>.
RocMarshal commented on code in PR #27903:
URL: https://github.com/apache/flink/pull/27903#discussion_r3067533110
##########
flink-docs/src/main/java/org/apache/flink/docs/rest/OpenApiSpecGenerator.java:
##########
@@ -279,15 +282,33 @@ private static void overrideIdSchemas(final OpenAPI openApi) {
.addSchemas(JobVertexID.class.getSimpleName(), idSchema)
.addSchemas(IntermediateDataSetID.class.getSimpleName(), idSchema)
.addSchemas(TriggerId.class.getSimpleName(), idSchema)
- .addSchemas(ResourceID.class.getSimpleName(), idSchema);
+ .addSchemas(ResourceID.class.getSimpleName(), idSchema)
+ .addSchemas(SlotSharingGroupId.class.getSimpleName(), idSchema);
Review Comment:
Nice fix!
##########
flink-docs/src/main/java/org/apache/flink/docs/rest/OpenApiSpecGenerator.java:
##########
@@ -279,15 +282,33 @@ private static void overrideIdSchemas(final OpenAPI openApi) {
.addSchemas(JobVertexID.class.getSimpleName(), idSchema)
.addSchemas(IntermediateDataSetID.class.getSimpleName(), idSchema)
.addSchemas(TriggerId.class.getSimpleName(), idSchema)
- .addSchemas(ResourceID.class.getSimpleName(), idSchema);
+ .addSchemas(ResourceID.class.getSimpleName(), idSchema)
+ .addSchemas(SlotSharingGroupId.class.getSimpleName(), idSchema);
+ }
+
+ /**
+ * Overrides the schema for {@link MetricCollectionResponseBody} which uses a custom Jackson
+ * serializer that writes the metrics collection as a raw JSON array, not as an object with a
+ * "metrics" field.
+ */
+ private static void overrideMetricCollectionSchema(final OpenAPI openApi) {
+ final ArraySchema metricArraySchema =
+ new ArraySchema().items(new Schema().$ref("#/components/schemas/Metric"));
+
+ openApi.getComponents()
+ .addSchemas(MetricCollectionResponseBody.class.getSimpleName(), metricArraySchema);
}
private static void overrideSerializeThrowableSchema(final OpenAPI openAPI) {
final Schema serializedThrowableSchema =
new Schema<>()
.type("object")
.properties(
- Collections.singletonMap(
+ Map.of(
+ SerializedThrowableSerializer.FIELD_NAME_CLASS,
+ new Schema().type("string"),
+ SerializedThrowableSerializer.FIELD_NAME_STACK_TRACE,
Review Comment:
👍
##########
flink-docs/src/main/java/org/apache/flink/docs/rest/OpenApiSpecGenerator.java:
##########
@@ -279,15 +282,33 @@ private static void overrideIdSchemas(final OpenAPI openApi) {
.addSchemas(JobVertexID.class.getSimpleName(), idSchema)
.addSchemas(IntermediateDataSetID.class.getSimpleName(), idSchema)
.addSchemas(TriggerId.class.getSimpleName(), idSchema)
- .addSchemas(ResourceID.class.getSimpleName(), idSchema);
+ .addSchemas(ResourceID.class.getSimpleName(), idSchema)
+ .addSchemas(SlotSharingGroupId.class.getSimpleName(), idSchema);
+ }
+
+ /**
+ * Overrides the schema for {@link MetricCollectionResponseBody} which uses a custom Jackson
+ * serializer that writes the metrics collection as a raw JSON array, not as an object with a
+ * "metrics" field.
+ */
+ private static void overrideMetricCollectionSchema(final OpenAPI openApi) {
Review Comment:
IIUC, The method is not need.
Because the `MetricCollectionResponseBody` is not a collection like `array` or `list`.
<img width="2434" height="1346" alt="Image" src="https://github.com/user-attachments/assets/cb7b9e13-c770-4fcd-ac18-100e29a9c407" />
If the definition of `MetricCollectionResponseBody` like follows, maybe the change is need.
<img width="2492" height="1774" alt="Image" src="https://github.com/user-attachments/assets/6229898b-4eff-4d0d-9e92-7db42911111e" />
Pls let me know your opinion.
--
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
Re: [PR] [hotfix][docs] Add SlotSharingGroupId to OpenAPI spec generator ID schema overrides [flink]
Posted by "nikhilsu (via GitHub)" <gi...@apache.org>.
nikhilsu commented on code in PR #27903:
URL: https://github.com/apache/flink/pull/27903#discussion_r3049993999
##########
docs/static/generated/rest_v1_dispatcher.yml:
##########
Review Comment:
FYI: changes to `*.yml` were made as part of running `./mvnw package -Dgenerate-rest-docs -pl flink-docs -am -nsu -DskipTests 2>&1`
--
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
Re: [PR] [FLINK-39422] [REST][docs] Fix OpenAPI spec for `SlotSharingGroupId`, `MetricCollectionResponseBody`, and `SerializedThrowable` custom serializers [flink]
Posted by "nikhilsu (via GitHub)" <gi...@apache.org>.
nikhilsu commented on code in PR #27903:
URL: https://github.com/apache/flink/pull/27903#discussion_r3069055601
##########
flink-docs/src/main/java/org/apache/flink/docs/rest/OpenApiSpecGenerator.java:
##########
@@ -279,15 +282,33 @@ private static void overrideIdSchemas(final OpenAPI openApi) {
.addSchemas(JobVertexID.class.getSimpleName(), idSchema)
.addSchemas(IntermediateDataSetID.class.getSimpleName(), idSchema)
.addSchemas(TriggerId.class.getSimpleName(), idSchema)
- .addSchemas(ResourceID.class.getSimpleName(), idSchema);
+ .addSchemas(ResourceID.class.getSimpleName(), idSchema)
+ .addSchemas(SlotSharingGroupId.class.getSimpleName(), idSchema);
+ }
+
+ /**
+ * Overrides the schema for {@link MetricCollectionResponseBody} which uses a custom Jackson
+ * serializer that writes the metrics collection as a raw JSON array, not as an object with a
+ * "metrics" field.
+ */
+ private static void overrideMetricCollectionSchema(final OpenAPI openApi) {
Review Comment:
I've pushed the change. Let me know what you think.
--
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
Re: [PR] [FLINK-39422] [REST][docs] Fix OpenAPI spec for `SlotSharingGroupId`, `MetricCollectionResponseBody`, and `SerializedThrowable` custom serializers [flink]
Posted by "nikhilsu (via GitHub)" <gi...@apache.org>.
nikhilsu commented on code in PR #27903:
URL: https://github.com/apache/flink/pull/27903#discussion_r3140718774
##########
flink-docs/src/main/java/org/apache/flink/docs/rest/OpenApiSpecGenerator.java:
##########
@@ -279,18 +280,23 @@ private static void overrideIdSchemas(final OpenAPI openApi) {
.addSchemas(JobVertexID.class.getSimpleName(), idSchema)
.addSchemas(IntermediateDataSetID.class.getSimpleName(), idSchema)
.addSchemas(TriggerId.class.getSimpleName(), idSchema)
- .addSchemas(ResourceID.class.getSimpleName(), idSchema);
+ .addSchemas(ResourceID.class.getSimpleName(), idSchema)
+ .addSchemas(SlotSharingGroupId.class.getSimpleName(), idSchema);
}
private static void overrideSerializeThrowableSchema(final OpenAPI openAPI) {
final Schema serializedThrowableSchema =
new Schema<>()
.type("object")
.properties(
- Collections.singletonMap(
+ Map.of(
+ SerializedThrowableSerializer.FIELD_NAME_CLASS,
+ new Schema().type("string"),
+ SerializedThrowableSerializer.FIELD_NAME_STACK_TRACE,
+ new Schema().type("string"),
SerializedThrowableSerializer
.FIELD_NAME_SERIALIZED_THROWABLE,
- new Schema().type("string").format("binary")));
+ new Schema().type("string").format("byte")));
Review Comment:
## Binary vs Byte formats
- `binary`: Represents raw (unencoded) binary data. Used when the HTTP body contains actual octets (e.g., [multipart file uploads](https://swagger.io/docs/specification/v3_0/describing-request-body/file-upload/)).
- `byte`: Represents Base64-encoded characters. Use this when you are sending binary data inside a JSON object (e.g., {"image_data": "SGVsbG8="}).
`format: byte` is the correct format for Base64-encoded data in JSON properties, which is what [gen.writeBinaryField(..)](https://github.com/apache/flink/blob/release-2.2/flink-runtime/src/main/java/org/apache/flink/runtime/rest/messages/json/SerializedThrowableSerializer.java#L54) produces.
--
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
Re: [PR] [FLINK-39422] [REST][docs] Fix OpenAPI spec for `SlotSharingGroupId`, `MetricCollectionResponseBody`, and `SerializedThrowable` custom serializers [flink]
Posted by "RocMarshal (via GitHub)" <gi...@apache.org>.
RocMarshal commented on code in PR #27903:
URL: https://github.com/apache/flink/pull/27903#discussion_r3076772418
##########
flink-docs/src/main/java/org/apache/flink/docs/rest/OpenApiSpecGenerator.java:
##########
@@ -279,15 +282,33 @@ private static void overrideIdSchemas(final OpenAPI openApi) {
.addSchemas(JobVertexID.class.getSimpleName(), idSchema)
.addSchemas(IntermediateDataSetID.class.getSimpleName(), idSchema)
.addSchemas(TriggerId.class.getSimpleName(), idSchema)
- .addSchemas(ResourceID.class.getSimpleName(), idSchema);
+ .addSchemas(ResourceID.class.getSimpleName(), idSchema)
+ .addSchemas(SlotSharingGroupId.class.getSimpleName(), idSchema);
+ }
+
+ /**
+ * Overrides the schema for {@link MetricCollectionResponseBody} which uses a custom Jackson
+ * serializer that writes the metrics collection as a raw JSON array, not as an object with a
+ * "metrics" field.
+ */
+ private static void overrideMetricCollectionSchema(final OpenAPI openApi) {
Review Comment:
> Hi, @nikhilsu In my limited reading,Perhaps implementing the `MetricCollectionResponseBody` class in the style of `org.apache.flink.runtime.rest.messages.ConfigurationInfo` would be a good choice. Main considerations are as follows:
>
> * We wouldn’t need to modify the `OpenApiSpecGenerator` class when introducing similar new classes in the future
> * There is already a precedent with the implementation of `ConfigurationInfo`, and we wouldn’t need to explicitly introduce serialization and deserialization processors
> * In the REST API’s HTML and YML documentation, the definition of the return type would be relatively more detailed. For example, an `object` type would be refined to something like `Array<Metric>`
Hi, @1996fanrui Could you help take a look for this strategy ?
I guess the generation of the REST API documentation might be related to the release quality of 2.3, although the impact is probably relatively small.
Thank you very much.
--
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
Re: [PR] [FLINK-39422] [REST][docs] Fix OpenAPI spec for `SlotSharingGroupId`, `MetricCollectionResponseBody`, and `SerializedThrowable` custom serializers [flink]
Posted by "nikhilsu (via GitHub)" <gi...@apache.org>.
nikhilsu commented on code in PR #27903:
URL: https://github.com/apache/flink/pull/27903#discussion_r3140716200
##########
flink-docs/src/main/java/org/apache/flink/docs/rest/OpenApiSpecGenerator.java:
##########
@@ -279,18 +280,23 @@ private static void overrideIdSchemas(final OpenAPI openApi) {
.addSchemas(JobVertexID.class.getSimpleName(), idSchema)
.addSchemas(IntermediateDataSetID.class.getSimpleName(), idSchema)
.addSchemas(TriggerId.class.getSimpleName(), idSchema)
- .addSchemas(ResourceID.class.getSimpleName(), idSchema);
+ .addSchemas(ResourceID.class.getSimpleName(), idSchema)
+ .addSchemas(SlotSharingGroupId.class.getSimpleName(), idSchema);
}
private static void overrideSerializeThrowableSchema(final OpenAPI openAPI) {
final Schema serializedThrowableSchema =
new Schema<>()
.type("object")
.properties(
- Collections.singletonMap(
+ Map.of(
+ SerializedThrowableSerializer.FIELD_NAME_CLASS,
+ new Schema().type("string"),
+ SerializedThrowableSerializer.FIELD_NAME_STACK_TRACE,
+ new Schema().type("string"),
SerializedThrowableSerializer
.FIELD_NAME_SERIALIZED_THROWABLE,
- new Schema().type("string").format("binary")));
+ new Schema().type("string").format("byte")));
openAPI.getComponents()
.addSchemas(SerializedThrowable.class.getSimpleName(), serializedThrowableSchema);
Review Comment:
## Binary vs Byte formats
- `binary`: Represents raw (unencoded) binary data. Used when the HTTP body contains actual octets (e.g., [multipart file uploads](https://swagger.io/docs/specification/v3_0/describing-request-body/file-upload/)).
- `byte`: Represents Base64-encoded characters. Use this when you are sending binary data inside a JSON object (e.g., {"image_data": "SGVsbG8="}).
`format: byte` is the correct format for Base64-encoded data in JSON properties, which is what [gen.writeBinaryField(..)](https://github.com/apache/flink/blob/release-2.2/flink-runtime/src/main/java/org/apache/flink/runtime/rest/messages/json/SerializedThrowableSerializer.java#L54) produces.
--
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
Re: [PR] [FLINK-39422] [REST][docs] Fix OpenAPI spec for `SlotSharingGroupId`, `MetricCollectionResponseBody`, and `SerializedThrowable` custom serializers [flink]
Posted by "nikhilsu (via GitHub)" <gi...@apache.org>.
nikhilsu commented on code in PR #27903:
URL: https://github.com/apache/flink/pull/27903#discussion_r3140716200
##########
flink-docs/src/main/java/org/apache/flink/docs/rest/OpenApiSpecGenerator.java:
##########
@@ -279,18 +280,23 @@ private static void overrideIdSchemas(final OpenAPI openApi) {
.addSchemas(JobVertexID.class.getSimpleName(), idSchema)
.addSchemas(IntermediateDataSetID.class.getSimpleName(), idSchema)
.addSchemas(TriggerId.class.getSimpleName(), idSchema)
- .addSchemas(ResourceID.class.getSimpleName(), idSchema);
+ .addSchemas(ResourceID.class.getSimpleName(), idSchema)
+ .addSchemas(SlotSharingGroupId.class.getSimpleName(), idSchema);
}
private static void overrideSerializeThrowableSchema(final OpenAPI openAPI) {
final Schema serializedThrowableSchema =
new Schema<>()
.type("object")
.properties(
- Collections.singletonMap(
+ Map.of(
+ SerializedThrowableSerializer.FIELD_NAME_CLASS,
+ new Schema().type("string"),
+ SerializedThrowableSerializer.FIELD_NAME_STACK_TRACE,
+ new Schema().type("string"),
SerializedThrowableSerializer
.FIELD_NAME_SERIALIZED_THROWABLE,
- new Schema().type("string").format("binary")));
+ new Schema().type("string").format("byte")));
openAPI.getComponents()
.addSchemas(SerializedThrowable.class.getSimpleName(), serializedThrowableSchema);
Review Comment:
## Binary vs Byte formats
- `binary`: Represents raw (unencoded) binary data. Used when the HTTP body contains actual octets (e.g., [multipart file uploads](https://swagger.io/docs/specification/v3_0/describing-request-body/file-upload/)).
- `byte`: Represents Base64-encoded characters. Use this when you are sending binary data inside a JSON object (e.g., {"image_data": "SGVsbG8="}).
`format: byte` is the correct format for Base64-encoded data in JSON properties, which is what [gen.writeBinaryField(..)](https://github.com/apache/flink/blob/release-2.2/flink-runtime/src/main/java/org/apache/flink/runtime/rest/messages/json/SerializedThrowableSerializer.java#L54) produces.
--
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
Re: [PR] [FLINK-39422] [REST][docs] Fix OpenAPI spec for `SlotSharingGroupId`, `MetricCollectionResponseBody`, and `SerializedThrowable` custom serializers [flink]
Posted by "nikhilsu (via GitHub)" <gi...@apache.org>.
nikhilsu commented on code in PR #27903:
URL: https://github.com/apache/flink/pull/27903#discussion_r3049993999
##########
docs/static/generated/rest_v1_dispatcher.yml:
##########
Review Comment:
FYI: changes to `*.yml` were made as part of running `./mvnw package -Dgenerate-rest-docs -pl flink-docs -am -nsu -DskipTests 2>&1`
--
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
Re: [PR] [FLINK-39422] [REST][docs] Fix OpenAPI spec for `SlotSharingGroupId`, `MetricCollectionResponseBody`, and `SerializedThrowable` custom serializers [flink]
Posted by "nikhilsu (via GitHub)" <gi...@apache.org>.
nikhilsu commented on PR #27903:
URL: https://github.com/apache/flink/pull/27903#issuecomment-4249517468
> Thanks @nikhilsu . LGTM on the whole, just left a few of comments. BTW, IIUC, `RuntimeRestAPIStabilityTest` will be failure, maybe you could have a try on running it. then do some adjustments based on the hint output of the unit-test result. like running `RuntimeRestAPIStabilityTest` with `-Dgenerate-rest-snapshot`
Ran `mvn test -pl flink-runtime-web -Dtest="RuntimeRestAPIStabilityTest" -Dgenerate-rest-snapshot -Denforcer.skip=true` and `mvn test -pl flink-runtime-web -Dtest="RuntimeRestAPIStabilityTest" -Denforcer.skip=true` and both pass.
The test pass because the REST snapshots records `"type": "any"` for metric endpoints because Jackson's `JsonSchemaGenerator` (used by [CompatibilityRoutines](https://github.com/apache/flink/blob/release-2.2/flink-runtime/src/test/java/org/apache/flink/runtime/rest/compatibility/CompatibilityRoutines.java#L188)) couldn't introspect the old custom serializer. The compatibility check https://github.com/apache/flink/blob/release-2.2/flink-runtime/src/test/java/org/apache/flink/runtime/rest/compatibility/CompatibilityRoutines.java#L218 when the old type is any, so the snapshot doesn't need updating.
--
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
Re: [PR] [FLINK-39422] [REST][docs] Fix OpenAPI spec for `SlotSharingGroupId`, `MetricCollectionResponseBody`, and `SerializedThrowable` custom serializers [flink]
Posted by "nikhilsu (via GitHub)" <gi...@apache.org>.
nikhilsu commented on code in PR #27903:
URL: https://github.com/apache/flink/pull/27903#discussion_r3083845822
##########
flink-runtime/src/main/java/org/apache/flink/runtime/rest/messages/job/metrics/MetricCollectionResponseBody.java:
##########
@@ -46,61 +37,26 @@
* <pre>{@code
* [{"id": "metricName", "value": "1"}]
* }</pre>
- *
- * @see Serializer
- * @see Deserializer
- * @see org.apache.flink.runtime.rest.handler.legacy.metrics.MetricStore
*/
-@JsonSerialize(using = MetricCollectionResponseBody.Serializer.class)
-@JsonDeserialize(using = MetricCollectionResponseBody.Deserializer.class)
-public final class MetricCollectionResponseBody implements ResponseBody {
+public final class MetricCollectionResponseBody extends ArrayList<Metric> implements ResponseBody {
+
+ private static final long serialVersionUID = -1170348873871206965L;
- private final Collection<Metric> metrics;
+ // a default constructor is required for collection type marshaling
+ public MetricCollectionResponseBody() {}
public MetricCollectionResponseBody(Collection<Metric> metrics) {
- this.metrics = requireNonNull(metrics, "metrics must not be null");
+ super(requireNonNull(metrics, "metrics must not be null"));
}
+ @JsonIgnore
public Collection<Metric> getMetrics() {
- return metrics;
- }
-
- /** JSON serializer for {@link MetricCollectionResponseBody}. */
- public static class Serializer extends StdSerializer<MetricCollectionResponseBody> {
-
- private static final long serialVersionUID = 1L;
-
- protected Serializer() {
- super(MetricCollectionResponseBody.class);
- }
-
- @Override
- public void serialize(
- MetricCollectionResponseBody metricCollectionResponseBody,
- JsonGenerator jsonGenerator,
- SerializerProvider serializerProvider)
- throws IOException {
-
- jsonGenerator.writeObject(metricCollectionResponseBody.getMetrics());
- }
+ return this;
Review Comment:
Actually let me make it simpler and remove this method altogether. It's mostly used only in tests.
--
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
Re: [PR] [FLINK-39422] [REST][docs] Fix OpenAPI spec for `SlotSharingGroupId`, `MetricCollectionResponseBody`, and `SerializedThrowable` custom serializers [flink]
Posted by "RocMarshal (via GitHub)" <gi...@apache.org>.
RocMarshal commented on code in PR #27903:
URL: https://github.com/apache/flink/pull/27903#discussion_r3084018311
##########
flink-runtime/src/main/java/org/apache/flink/runtime/rest/messages/job/metrics/MetricCollectionResponseBody.java:
##########
@@ -46,61 +37,26 @@
* <pre>{@code
* [{"id": "metricName", "value": "1"}]
* }</pre>
- *
- * @see Serializer
- * @see Deserializer
- * @see org.apache.flink.runtime.rest.handler.legacy.metrics.MetricStore
*/
-@JsonSerialize(using = MetricCollectionResponseBody.Serializer.class)
-@JsonDeserialize(using = MetricCollectionResponseBody.Deserializer.class)
-public final class MetricCollectionResponseBody implements ResponseBody {
+public final class MetricCollectionResponseBody extends ArrayList<Metric> implements ResponseBody {
+
+ private static final long serialVersionUID = -1170348873871206965L;
- private final Collection<Metric> metrics;
+ // a default constructor is required for collection type marshaling
+ public MetricCollectionResponseBody() {}
public MetricCollectionResponseBody(Collection<Metric> metrics) {
- this.metrics = requireNonNull(metrics, "metrics must not be null");
+ super(requireNonNull(metrics, "metrics must not be null"));
}
+ @JsonIgnore
public Collection<Metric> getMetrics() {
- return metrics;
- }
-
- /** JSON serializer for {@link MetricCollectionResponseBody}. */
- public static class Serializer extends StdSerializer<MetricCollectionResponseBody> {
-
- private static final long serialVersionUID = 1L;
-
- protected Serializer() {
- super(MetricCollectionResponseBody.class);
- }
-
- @Override
- public void serialize(
- MetricCollectionResponseBody metricCollectionResponseBody,
- JsonGenerator jsonGenerator,
- SerializerProvider serializerProvider)
- throws IOException {
-
- jsonGenerator.writeObject(metricCollectionResponseBody.getMetrics());
- }
+ return this;
Review Comment:
@nikhilsu One of the reasons for keeping this method is to maintain compatibility with previous behavior as much as possible.
--
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
Re: [PR] [FLINK-39422] [REST][docs] Fix OpenAPI spec for `SlotSharingGroupId`, `MetricCollectionResponseBody`, and `SerializedThrowable` custom serializers [flink]
Posted by "nikhilsu (via GitHub)" <gi...@apache.org>.
nikhilsu commented on PR #27903:
URL: https://github.com/apache/flink/pull/27903#issuecomment-4227299037
@RocMarshal do you know who can review this PR?
--
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
Re: [PR] [FLINK-39422] [REST][docs] Fix OpenAPI spec for `SlotSharingGroupId`, `MetricCollectionResponseBody`, and `SerializedThrowable` custom serializers [flink]
Posted by "RocMarshal (via GitHub)" <gi...@apache.org>.
RocMarshal commented on code in PR #27903:
URL: https://github.com/apache/flink/pull/27903#discussion_r3068205221
##########
flink-docs/src/main/java/org/apache/flink/docs/rest/OpenApiSpecGenerator.java:
##########
@@ -279,15 +282,33 @@ private static void overrideIdSchemas(final OpenAPI openApi) {
.addSchemas(JobVertexID.class.getSimpleName(), idSchema)
.addSchemas(IntermediateDataSetID.class.getSimpleName(), idSchema)
.addSchemas(TriggerId.class.getSimpleName(), idSchema)
- .addSchemas(ResourceID.class.getSimpleName(), idSchema);
+ .addSchemas(ResourceID.class.getSimpleName(), idSchema)
+ .addSchemas(SlotSharingGroupId.class.getSimpleName(), idSchema);
+ }
+
+ /**
+ * Overrides the schema for {@link MetricCollectionResponseBody} which uses a custom Jackson
+ * serializer that writes the metrics collection as a raw JSON array, not as an object with a
+ * "metrics" field.
+ */
+ private static void overrideMetricCollectionSchema(final OpenAPI openApi) {
Review Comment:
Hi, @nikhilsu
In my limited reading,Perhaps implementing the `MetricCollectionResponseBody` class in the style of
`org.apache.flink.runtime.rest.messages.ConfigurationInfo` would be a good choice.
Main considerations are as follows:
- We wouldn’t need to modify the `OpenApiSpecGenerator` class when introducing similar new classes in the future
- There is already a precedent with the implementation of `ConfigurationInfo`, and we wouldn’t need to explicitly introduce serialization and deserialization processors
- In the REST API’s HTML and YML documentation, the definition of the return type would be relatively more detailed. For example, an `object` type would be refined to something like `Array<Metric>`
- this should not break the existing JSON field structure.
--
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
Re: [PR] [hotfix][docs] Add SlotSharingGroupId to OpenAPI spec generator ID schema overrides [flink]
Posted by "nikhilsu (via GitHub)" <gi...@apache.org>.
nikhilsu commented on PR #27903:
URL: https://github.com/apache/flink/pull/27903#issuecomment-4204751537
cc: @zentol @gaborgsomogyi
--
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
Re: [PR] [FLINK-39422] [REST][docs] Fix OpenAPI spec for `SlotSharingGroupId`, `MetricCollectionResponseBody`, and `SerializedThrowable` custom serializers [flink]
Posted by "RocMarshal (via GitHub)" <gi...@apache.org>.
RocMarshal commented on code in PR #27903:
URL: https://github.com/apache/flink/pull/27903#discussion_r3083632274
##########
flink-runtime/src/main/java/org/apache/flink/runtime/rest/messages/job/metrics/MetricCollectionResponseBody.java:
##########
@@ -20,19 +20,10 @@
import org.apache.flink.runtime.rest.messages.ResponseBody;
-import org.apache.flink.shaded.jackson2.com.fasterxml.jackson.core.JsonGenerator;
-import org.apache.flink.shaded.jackson2.com.fasterxml.jackson.core.JsonParser;
-import org.apache.flink.shaded.jackson2.com.fasterxml.jackson.core.type.TypeReference;
-import org.apache.flink.shaded.jackson2.com.fasterxml.jackson.databind.DeserializationContext;
-import org.apache.flink.shaded.jackson2.com.fasterxml.jackson.databind.SerializerProvider;
-import org.apache.flink.shaded.jackson2.com.fasterxml.jackson.databind.annotation.JsonDeserialize;
-import org.apache.flink.shaded.jackson2.com.fasterxml.jackson.databind.annotation.JsonSerialize;
-import org.apache.flink.shaded.jackson2.com.fasterxml.jackson.databind.deser.std.StdDeserializer;
-import org.apache.flink.shaded.jackson2.com.fasterxml.jackson.databind.ser.std.StdSerializer;
+import org.apache.flink.shaded.jackson2.com.fasterxml.jackson.annotation.JsonIgnore;
-import java.io.IOException;
+import java.util.ArrayList;
import java.util.Collection;
Review Comment:
```suggestion
import java.util.Collection;
import java.util.List;
```
##########
flink-runtime/src/main/java/org/apache/flink/runtime/rest/messages/job/metrics/MetricCollectionResponseBody.java:
##########
@@ -46,61 +37,26 @@
* <pre>{@code
* [{"id": "metricName", "value": "1"}]
* }</pre>
- *
- * @see Serializer
- * @see Deserializer
- * @see org.apache.flink.runtime.rest.handler.legacy.metrics.MetricStore
*/
-@JsonSerialize(using = MetricCollectionResponseBody.Serializer.class)
-@JsonDeserialize(using = MetricCollectionResponseBody.Deserializer.class)
-public final class MetricCollectionResponseBody implements ResponseBody {
+public final class MetricCollectionResponseBody extends ArrayList<Metric> implements ResponseBody {
+
+ private static final long serialVersionUID = -1170348873871206965L;
- private final Collection<Metric> metrics;
+ // a default constructor is required for collection type marshaling
+ public MetricCollectionResponseBody() {}
public MetricCollectionResponseBody(Collection<Metric> metrics) {
- this.metrics = requireNonNull(metrics, "metrics must not be null");
+ super(requireNonNull(metrics, "metrics must not be null"));
}
+ @JsonIgnore
public Collection<Metric> getMetrics() {
- return metrics;
- }
-
- /** JSON serializer for {@link MetricCollectionResponseBody}. */
- public static class Serializer extends StdSerializer<MetricCollectionResponseBody> {
-
- private static final long serialVersionUID = 1L;
-
- protected Serializer() {
- super(MetricCollectionResponseBody.class);
- }
-
- @Override
- public void serialize(
- MetricCollectionResponseBody metricCollectionResponseBody,
- JsonGenerator jsonGenerator,
- SerializerProvider serializerProvider)
- throws IOException {
-
- jsonGenerator.writeObject(metricCollectionResponseBody.getMetrics());
- }
+ return this;
Review Comment:
```suggestion
return List.copyOf(this);
```
##########
flink-runtime/src/main/java/org/apache/flink/runtime/rest/messages/job/metrics/MetricCollectionResponseBody.java:
##########
@@ -46,61 +37,26 @@
* <pre>{@code
* [{"id": "metricName", "value": "1"}]
* }</pre>
Review Comment:
```suggestion
* }</pre>
*
* @see org.apache.flink.runtime.rest.handler.legacy.metrics.MetricStore
```
##########
flink-runtime/src/main/java/org/apache/flink/runtime/rest/messages/job/metrics/MetricCollectionResponseBody.java:
##########
@@ -46,61 +37,26 @@
* <pre>{@code
* [{"id": "metricName", "value": "1"}]
* }</pre>
- *
- * @see Serializer
- * @see Deserializer
- * @see org.apache.flink.runtime.rest.handler.legacy.metrics.MetricStore
*/
-@JsonSerialize(using = MetricCollectionResponseBody.Serializer.class)
-@JsonDeserialize(using = MetricCollectionResponseBody.Deserializer.class)
-public final class MetricCollectionResponseBody implements ResponseBody {
+public final class MetricCollectionResponseBody extends ArrayList<Metric> implements ResponseBody {
+
+ private static final long serialVersionUID = -1170348873871206965L;
- private final Collection<Metric> metrics;
+ // a default constructor is required for collection type marshaling
Review Comment:
```suggestion
// a default constructor is required for collection type marshalling
```
--
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
Re: [PR] [FLINK-39422] [REST][docs] Fix OpenAPI spec for `SlotSharingGroupId`, `MetricCollectionResponseBody`, and `SerializedThrowable` custom serializers [flink]
Posted by "nikhilsu (via GitHub)" <gi...@apache.org>.
nikhilsu commented on code in PR #27903:
URL: https://github.com/apache/flink/pull/27903#discussion_r3083851357
##########
flink-docs/src/main/java/org/apache/flink/docs/rest/OpenApiSpecGenerator.java:
##########
@@ -279,15 +282,33 @@ private static void overrideIdSchemas(final OpenAPI openApi) {
.addSchemas(JobVertexID.class.getSimpleName(), idSchema)
.addSchemas(IntermediateDataSetID.class.getSimpleName(), idSchema)
.addSchemas(TriggerId.class.getSimpleName(), idSchema)
- .addSchemas(ResourceID.class.getSimpleName(), idSchema);
+ .addSchemas(ResourceID.class.getSimpleName(), idSchema)
+ .addSchemas(SlotSharingGroupId.class.getSimpleName(), idSchema);
+ }
+
+ /**
+ * Overrides the schema for {@link MetricCollectionResponseBody} which uses a custom Jackson
+ * serializer that writes the metrics collection as a raw JSON array, not as an object with a
+ * "metrics" field.
+ */
+ private static void overrideMetricCollectionSchema(final OpenAPI openApi) {
Review Comment:
Btw, @RocMarshal now that we've updated `MetricCollectionResponseBody` to mirror the structure of `ConfigurationInfo`, for the sake of consistency we should do the same in case of:
[AggregatedMetricsResponseBody.java](https://github.com/apache/flink/blob/release-2.2/flink-runtime/src/main/java/org/apache/flink/runtime/rest/messages/job/metrics/AggregatedMetricsResponseBody.java#L55)
--
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
Re: [PR] [FLINK-39422] [REST][docs] Fix OpenAPI spec for `SlotSharingGroupId`, `MetricCollectionResponseBody`, and `SerializedThrowable` custom serializers [flink]
Posted by "RocMarshal (via GitHub)" <gi...@apache.org>.
RocMarshal commented on code in PR #27903:
URL: https://github.com/apache/flink/pull/27903#discussion_r3068205221
##########
flink-docs/src/main/java/org/apache/flink/docs/rest/OpenApiSpecGenerator.java:
##########
@@ -279,15 +282,33 @@ private static void overrideIdSchemas(final OpenAPI openApi) {
.addSchemas(JobVertexID.class.getSimpleName(), idSchema)
.addSchemas(IntermediateDataSetID.class.getSimpleName(), idSchema)
.addSchemas(TriggerId.class.getSimpleName(), idSchema)
- .addSchemas(ResourceID.class.getSimpleName(), idSchema);
+ .addSchemas(ResourceID.class.getSimpleName(), idSchema)
+ .addSchemas(SlotSharingGroupId.class.getSimpleName(), idSchema);
+ }
+
+ /**
+ * Overrides the schema for {@link MetricCollectionResponseBody} which uses a custom Jackson
+ * serializer that writes the metrics collection as a raw JSON array, not as an object with a
+ * "metrics" field.
+ */
+ private static void overrideMetricCollectionSchema(final OpenAPI openApi) {
Review Comment:
Hi, @nikhilsu
In my limited reading,Perhaps implementing the `MetricCollectionResponseBody` class in the style of
`org.apache.flink.runtime.rest.messages.ConfigurationInfo` would be a good choice.
Main considerations are as follows:
- We wouldn’t need to modify the `OpenApiSpecGenerator` class when introducing similar new classes in the future
- There is already a precedent with the implementation of `ConfigurationInfo`, and we wouldn’t need to explicitly introduce serialization and deserialization processors
- In the REST API’s HTML and YML documentation, the definition of the return type would be relatively more detailed. For example, an object type would be refined to something like `Array<Metric>`
##########
flink-docs/src/main/java/org/apache/flink/docs/rest/OpenApiSpecGenerator.java:
##########
@@ -279,15 +282,33 @@ private static void overrideIdSchemas(final OpenAPI openApi) {
.addSchemas(JobVertexID.class.getSimpleName(), idSchema)
.addSchemas(IntermediateDataSetID.class.getSimpleName(), idSchema)
.addSchemas(TriggerId.class.getSimpleName(), idSchema)
- .addSchemas(ResourceID.class.getSimpleName(), idSchema);
+ .addSchemas(ResourceID.class.getSimpleName(), idSchema)
+ .addSchemas(SlotSharingGroupId.class.getSimpleName(), idSchema);
+ }
+
+ /**
+ * Overrides the schema for {@link MetricCollectionResponseBody} which uses a custom Jackson
+ * serializer that writes the metrics collection as a raw JSON array, not as an object with a
+ * "metrics" field.
+ */
+ private static void overrideMetricCollectionSchema(final OpenAPI openApi) {
Review Comment:
Hi, @nikhilsu
In my limited reading,Perhaps implementing the `MetricCollectionResponseBody` class in the style of
`org.apache.flink.runtime.rest.messages.ConfigurationInfo` would be a good choice.
Main considerations are as follows:
- We wouldn’t need to modify the `OpenApiSpecGenerator` class when introducing similar new classes in the future
- There is already a precedent with the implementation of `ConfigurationInfo`, and we wouldn’t need to explicitly introduce serialization and deserialization processors
- In the REST API’s HTML and YML documentation, the definition of the return type would be relatively more detailed. For example, an `object` type would be refined to something like `Array<Metric>`
--
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
Re: [PR] [FLINK-39422] [REST][docs] Fix OpenAPI spec for `SlotSharingGroupId`, `MetricCollectionResponseBody`, and `SerializedThrowable` custom serializers [flink]
Posted by "RocMarshal (via GitHub)" <gi...@apache.org>.
RocMarshal commented on code in PR #27903:
URL: https://github.com/apache/flink/pull/27903#discussion_r3076772418
##########
flink-docs/src/main/java/org/apache/flink/docs/rest/OpenApiSpecGenerator.java:
##########
@@ -279,15 +282,33 @@ private static void overrideIdSchemas(final OpenAPI openApi) {
.addSchemas(JobVertexID.class.getSimpleName(), idSchema)
.addSchemas(IntermediateDataSetID.class.getSimpleName(), idSchema)
.addSchemas(TriggerId.class.getSimpleName(), idSchema)
- .addSchemas(ResourceID.class.getSimpleName(), idSchema);
+ .addSchemas(ResourceID.class.getSimpleName(), idSchema)
+ .addSchemas(SlotSharingGroupId.class.getSimpleName(), idSchema);
+ }
+
+ /**
+ * Overrides the schema for {@link MetricCollectionResponseBody} which uses a custom Jackson
+ * serializer that writes the metrics collection as a raw JSON array, not as an object with a
+ * "metrics" field.
+ */
+ private static void overrideMetricCollectionSchema(final OpenAPI openApi) {
Review Comment:
> Hi, @nikhilsu In my limited reading,Perhaps implementing the `MetricCollectionResponseBody` class in the style of `org.apache.flink.runtime.rest.messages.ConfigurationInfo` would be a good choice. Main considerations are as follows:
>
> * We wouldn’t need to modify the `OpenApiSpecGenerator` class when introducing similar new classes in the future
> * There is already a precedent with the implementation of `ConfigurationInfo`, and we wouldn’t need to explicitly introduce serialization and deserialization processors
> * In the REST API’s HTML and YML documentation, the definition of the return type would be relatively more detailed. For example, an `object` type would be refined to something like `Array<Metric>`
Hi, @1996fanrui Could you help take a look for this strategy ?
I guess the generation of the REST API documentation might be related to the release quality of 2.3, although the impact is probably relatively small.
Thank you very much.
--
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
Re: [PR] [FLINK-39422] [REST][docs] Fix OpenAPI spec for `SlotSharingGroupId`, `MetricCollectionResponseBody`, and `SerializedThrowable` custom serializers [flink]
Posted by "nikhilsu (via GitHub)" <gi...@apache.org>.
nikhilsu commented on PR #27903:
URL: https://github.com/apache/flink/pull/27903#issuecomment-4271897271
@RocMarshal any updates of this PR?
--
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
Re: [PR] [FLINK-39422] [REST][docs] Fix OpenAPI spec for `SlotSharingGroupId`, `MetricCollectionResponseBody`, and `SerializedThrowable` custom serializers [flink]
Posted by "RocMarshal (via GitHub)" <gi...@apache.org>.
RocMarshal commented on code in PR #27903:
URL: https://github.com/apache/flink/pull/27903#discussion_r3067819304
##########
flink-docs/src/main/java/org/apache/flink/docs/rest/OpenApiSpecGenerator.java:
##########
@@ -279,15 +282,33 @@ private static void overrideIdSchemas(final OpenAPI openApi) {
.addSchemas(JobVertexID.class.getSimpleName(), idSchema)
.addSchemas(IntermediateDataSetID.class.getSimpleName(), idSchema)
.addSchemas(TriggerId.class.getSimpleName(), idSchema)
- .addSchemas(ResourceID.class.getSimpleName(), idSchema);
+ .addSchemas(ResourceID.class.getSimpleName(), idSchema)
+ .addSchemas(SlotSharingGroupId.class.getSimpleName(), idSchema);
+ }
+
+ /**
+ * Overrides the schema for {@link MetricCollectionResponseBody} which uses a custom Jackson
+ * serializer that writes the metrics collection as a raw JSON array, not as an object with a
+ * "metrics" field.
+ */
+ private static void overrideMetricCollectionSchema(final OpenAPI openApi) {
Review Comment:
Sorry for ignoring the details and Thank you @nikhilsu for your clarification.
Got it~
--
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
Re: [PR] [FLINK-39422] [REST][docs] Fix OpenAPI spec for `SlotSharingGroupId`, `MetricCollectionResponseBody`, and `SerializedThrowable` custom serializers [flink]
Posted by "nikhilsu (via GitHub)" <gi...@apache.org>.
nikhilsu commented on code in PR #27903:
URL: https://github.com/apache/flink/pull/27903#discussion_r3069051959
##########
flink-docs/src/main/java/org/apache/flink/docs/rest/OpenApiSpecGenerator.java:
##########
@@ -279,15 +282,33 @@ private static void overrideIdSchemas(final OpenAPI openApi) {
.addSchemas(JobVertexID.class.getSimpleName(), idSchema)
.addSchemas(IntermediateDataSetID.class.getSimpleName(), idSchema)
.addSchemas(TriggerId.class.getSimpleName(), idSchema)
- .addSchemas(ResourceID.class.getSimpleName(), idSchema);
+ .addSchemas(ResourceID.class.getSimpleName(), idSchema)
+ .addSchemas(SlotSharingGroupId.class.getSimpleName(), idSchema);
+ }
+
+ /**
+ * Overrides the schema for {@link MetricCollectionResponseBody} which uses a custom Jackson
+ * serializer that writes the metrics collection as a raw JSON array, not as an object with a
+ * "metrics" field.
+ */
+ private static void overrideMetricCollectionSchema(final OpenAPI openApi) {
Review Comment:
Hey @RocMarshal, this is a good suggestion. It would be much cleaner to extend `MetricCollectionResponseBody` from an `ArrayList<Metric>`.
However, after this change, endpoints no longer reference `MetricCollectionResponseBody` - they inline the array. Generated clients would need regeneration. The wire format is the same (JSON array), but the type in generated code changes from `MetricCollectionResponseBody` to `List[Metric]`.
Here is the diff of the spec.
<img width="689" height="516" alt="Image" src="https://github.com/user-attachments/assets/2277b5a6-6c45-44eb-ad83-ed00f159e296" />
--
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
Re: [PR] [FLINK-39422] [REST][docs] Fix OpenAPI spec for `SlotSharingGroupId`, `MetricCollectionResponseBody`, and `SerializedThrowable` custom serializers [flink]
Posted by "nikhilsu (via GitHub)" <gi...@apache.org>.
nikhilsu commented on code in PR #27903:
URL: https://github.com/apache/flink/pull/27903#discussion_r3069054378
##########
flink-docs/src/main/java/org/apache/flink/docs/rest/OpenApiSpecGenerator.java:
##########
@@ -279,15 +282,33 @@ private static void overrideIdSchemas(final OpenAPI openApi) {
.addSchemas(JobVertexID.class.getSimpleName(), idSchema)
.addSchemas(IntermediateDataSetID.class.getSimpleName(), idSchema)
.addSchemas(TriggerId.class.getSimpleName(), idSchema)
- .addSchemas(ResourceID.class.getSimpleName(), idSchema);
+ .addSchemas(ResourceID.class.getSimpleName(), idSchema)
+ .addSchemas(SlotSharingGroupId.class.getSimpleName(), idSchema);
+ }
+
+ /**
+ * Overrides the schema for {@link MetricCollectionResponseBody} which uses a custom Jackson
+ * serializer that writes the metrics collection as a raw JSON array, not as an object with a
+ * "metrics" field.
+ */
+ private static void overrideMetricCollectionSchema(final OpenAPI openApi) {
Review Comment:
This would break downstream client code. But it might be ok as this endpoint would have never worked in the first place.
--
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
Re: [PR] [FLINK-39422] [REST][docs] Fix OpenAPI spec for `SlotSharingGroupId`, `MetricCollectionResponseBody`, and `SerializedThrowable` custom serializers [flink]
Posted by "nikhilsu (via GitHub)" <gi...@apache.org>.
nikhilsu commented on code in PR #27903:
URL: https://github.com/apache/flink/pull/27903#discussion_r3083851357
##########
flink-docs/src/main/java/org/apache/flink/docs/rest/OpenApiSpecGenerator.java:
##########
@@ -279,15 +282,33 @@ private static void overrideIdSchemas(final OpenAPI openApi) {
.addSchemas(JobVertexID.class.getSimpleName(), idSchema)
.addSchemas(IntermediateDataSetID.class.getSimpleName(), idSchema)
.addSchemas(TriggerId.class.getSimpleName(), idSchema)
- .addSchemas(ResourceID.class.getSimpleName(), idSchema);
+ .addSchemas(ResourceID.class.getSimpleName(), idSchema)
+ .addSchemas(SlotSharingGroupId.class.getSimpleName(), idSchema);
+ }
+
+ /**
+ * Overrides the schema for {@link MetricCollectionResponseBody} which uses a custom Jackson
+ * serializer that writes the metrics collection as a raw JSON array, not as an object with a
+ * "metrics" field.
+ */
+ private static void overrideMetricCollectionSchema(final OpenAPI openApi) {
Review Comment:
Btw, @RocMarshal now that we've updated `MetricCollectionResponseBody` to mirror the structure of `ConfigurationInfo`, I think we should also do the same for:
https://github.com/apache/flink/blob/release-2.2/flink-runtime/src/main/java/org/apache/flink/runtime/rest/messages/job/metrics/AggregatedMetricsResponseBody.java#L55
--
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
Re: [PR] [FLINK-39422] [REST][docs] Fix OpenAPI spec for `SlotSharingGroupId`, `MetricCollectionResponseBody`, and `SerializedThrowable` custom serializers [flink]
Posted by "nikhilsu (via GitHub)" <gi...@apache.org>.
nikhilsu commented on code in PR #27903:
URL: https://github.com/apache/flink/pull/27903#discussion_r3084034471
##########
flink-docs/src/main/java/org/apache/flink/docs/rest/OpenApiSpecGenerator.java:
##########
@@ -279,15 +282,33 @@ private static void overrideIdSchemas(final OpenAPI openApi) {
.addSchemas(JobVertexID.class.getSimpleName(), idSchema)
.addSchemas(IntermediateDataSetID.class.getSimpleName(), idSchema)
.addSchemas(TriggerId.class.getSimpleName(), idSchema)
- .addSchemas(ResourceID.class.getSimpleName(), idSchema);
+ .addSchemas(ResourceID.class.getSimpleName(), idSchema)
+ .addSchemas(SlotSharingGroupId.class.getSimpleName(), idSchema);
+ }
+
+ /**
+ * Overrides the schema for {@link MetricCollectionResponseBody} which uses a custom Jackson
+ * serializer that writes the metrics collection as a raw JSON array, not as an object with a
+ * "metrics" field.
+ */
+ private static void overrideMetricCollectionSchema(final OpenAPI openApi) {
Review Comment:
I'm pretty sure it does as it uses a custom serializer and deserializer.
--
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
Re: [PR] [FLINK-39422] [REST][docs] Fix OpenAPI spec for `SlotSharingGroupId`, `MetricCollectionResponseBody`, and `SerializedThrowable` custom serializers [flink]
Posted by "nikhilsu (via GitHub)" <gi...@apache.org>.
nikhilsu commented on code in PR #27903:
URL: https://github.com/apache/flink/pull/27903#discussion_r3069056496
##########
docs/layouts/shortcodes/generated/rest_v1_dispatcher.html:
##########
Review Comment:
FYI: changes to `*.html` and `*.yml` were made as part of running `./mvnw package -Dgenerate-rest-docs -pl flink-docs -am -nsu -DskipTests 2>&1`
--
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
Re: [PR] [FLINK-39422] [REST][docs] Fix OpenAPI spec for `SlotSharingGroupId`, `MetricCollectionResponseBody`, and `SerializedThrowable` custom serializers [flink]
Posted by "nikhilsu (via GitHub)" <gi...@apache.org>.
nikhilsu commented on code in PR #27903:
URL: https://github.com/apache/flink/pull/27903#discussion_r3067693048
##########
flink-docs/src/main/java/org/apache/flink/docs/rest/OpenApiSpecGenerator.java:
##########
@@ -279,15 +282,33 @@ private static void overrideIdSchemas(final OpenAPI openApi) {
.addSchemas(JobVertexID.class.getSimpleName(), idSchema)
.addSchemas(IntermediateDataSetID.class.getSimpleName(), idSchema)
.addSchemas(TriggerId.class.getSimpleName(), idSchema)
- .addSchemas(ResourceID.class.getSimpleName(), idSchema);
+ .addSchemas(ResourceID.class.getSimpleName(), idSchema)
+ .addSchemas(SlotSharingGroupId.class.getSimpleName(), idSchema);
+ }
+
+ /**
+ * Overrides the schema for {@link MetricCollectionResponseBody} which uses a custom Jackson
+ * serializer that writes the metrics collection as a raw JSON array, not as an object with a
+ * "metrics" field.
+ */
+ private static void overrideMetricCollectionSchema(final OpenAPI openApi) {
Review Comment:
Hey @RocMarshal thanks for the review!
The reason you need this is because `MetricCollectionResponseBody` uses a custom [Serializer](https://github.com/apache/flink/blob/release-1.20/flink-runtime/src/main/java/org/apache/flink/runtime/rest/messages/job/metrics/MetricCollectionResponseBody.java#L84) which omits the `metric` key and serializes a response object as a JSON array.
Here's how a sample response looks like if you call the `GET /v1/jobs/${jobid}/metrics` endpoint. Notice, that there is not `metric` key here, `MetricCollectionResponseBody` is serialized to a JSON array:
```console
$ curl 'http://localhost:8081/v1/jobs/bc6df938e80006c05639a139e6cf716f/metrics?get=numRecordsInPerSecond,numRecordsOutPerSecond,numRestarts' \
-H 'Accept: application/json, text/plain, */*' \
-H 'Accept-Language: en-US,en;q=0.9' \
--insecure | jq .
[
{
"id": "numRestarts",
"value": "0"
}
]
```
However, the problem is that the OpenAPI Spec Generator uses the class schema (through reflections) and generates API specs where the response includes the `metric` key: https://github.com/apache/flink/blob/release-1.20/docs/static/generated/rest_v1_dispatcher.yml#L2721-L2727
<img width="403" height="210" alt="Screenshot 2026-04-10 at 11 23 29 PM" src="https://github.com/user-attachments/assets/01e76076-b027-4155-b03e-61a47c91c485" />
This causes a mismatch in OpenAPI spec generated Flink clients. The client expects a `metric` key in the response but it actually does not exist.
--
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
Re: [PR] [FLINK-39422] [REST][docs] Fix OpenAPI spec for `SlotSharingGroupId`, `MetricCollectionResponseBody`, and `SerializedThrowable` custom serializers [flink]
Posted by "RocMarshal (via GitHub)" <gi...@apache.org>.
RocMarshal commented on code in PR #27903:
URL: https://github.com/apache/flink/pull/27903#discussion_r3068205221
##########
flink-docs/src/main/java/org/apache/flink/docs/rest/OpenApiSpecGenerator.java:
##########
@@ -279,15 +282,33 @@ private static void overrideIdSchemas(final OpenAPI openApi) {
.addSchemas(JobVertexID.class.getSimpleName(), idSchema)
.addSchemas(IntermediateDataSetID.class.getSimpleName(), idSchema)
.addSchemas(TriggerId.class.getSimpleName(), idSchema)
- .addSchemas(ResourceID.class.getSimpleName(), idSchema);
+ .addSchemas(ResourceID.class.getSimpleName(), idSchema)
+ .addSchemas(SlotSharingGroupId.class.getSimpleName(), idSchema);
+ }
+
+ /**
+ * Overrides the schema for {@link MetricCollectionResponseBody} which uses a custom Jackson
+ * serializer that writes the metrics collection as a raw JSON array, not as an object with a
+ * "metrics" field.
+ */
+ private static void overrideMetricCollectionSchema(final OpenAPI openApi) {
Review Comment:
In my limited reading,Perhaps implementing the MetricCollectionResponseBodyclass in the style of
org.apache.flink.runtime.rest.messages.ConfigurationInfowould be a good choice.
Main considerations are as follows:
- We wouldn’t need to modify the OpenApiSpecGeneratorclass when introducing similar new classes in the future
- There is already a precedent with the implementation of ConfigurationInfo, and we wouldn’t need to explicitly introduce serialization and deserialization processors
- In the REST API’s HTML and YML documentation, the definition of the return type would be relatively more detailed. For example, an object type would be refined to something like `Array<Metric>`
--
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
Re: [PR] [FLINK-39422] [REST][docs] Fix OpenAPI spec for `SlotSharingGroupId`, `MetricCollectionResponseBody`, and `SerializedThrowable` custom serializers [flink]
Posted by "RocMarshal (via GitHub)" <gi...@apache.org>.
RocMarshal commented on code in PR #27903:
URL: https://github.com/apache/flink/pull/27903#discussion_r3084025983
##########
flink-docs/src/main/java/org/apache/flink/docs/rest/OpenApiSpecGenerator.java:
##########
@@ -279,15 +282,33 @@ private static void overrideIdSchemas(final OpenAPI openApi) {
.addSchemas(JobVertexID.class.getSimpleName(), idSchema)
.addSchemas(IntermediateDataSetID.class.getSimpleName(), idSchema)
.addSchemas(TriggerId.class.getSimpleName(), idSchema)
- .addSchemas(ResourceID.class.getSimpleName(), idSchema);
+ .addSchemas(ResourceID.class.getSimpleName(), idSchema)
+ .addSchemas(SlotSharingGroupId.class.getSimpleName(), idSchema);
+ }
+
+ /**
+ * Overrides the schema for {@link MetricCollectionResponseBody} which uses a custom Jackson
+ * serializer that writes the metrics collection as a raw JSON array, not as an object with a
+ * "metrics" field.
+ */
+ private static void overrideMetricCollectionSchema(final OpenAPI openApi) {
Review Comment:
Hi， @nikhilsu
If the current implementation in this type of file does not cause the previous issues when generating the corresponding documentation, then I would prefer to keep it as is.
WDYTA?
--
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
Re: [PR] [FLINK-39422] [REST][docs] Fix OpenAPI spec for `SlotSharingGroupId`, `MetricCollectionResponseBody`, and `SerializedThrowable` custom serializers [flink]
Posted by "nikhilsu (via GitHub)" <gi...@apache.org>.
nikhilsu commented on code in PR #27903:
URL: https://github.com/apache/flink/pull/27903#discussion_r3084048217
##########
flink-docs/src/main/java/org/apache/flink/docs/rest/OpenApiSpecGenerator.java:
##########
@@ -279,15 +282,33 @@ private static void overrideIdSchemas(final OpenAPI openApi) {
.addSchemas(JobVertexID.class.getSimpleName(), idSchema)
.addSchemas(IntermediateDataSetID.class.getSimpleName(), idSchema)
.addSchemas(TriggerId.class.getSimpleName(), idSchema)
- .addSchemas(ResourceID.class.getSimpleName(), idSchema);
+ .addSchemas(ResourceID.class.getSimpleName(), idSchema)
+ .addSchemas(SlotSharingGroupId.class.getSimpleName(), idSchema);
+ }
+
+ /**
+ * Overrides the schema for {@link MetricCollectionResponseBody} which uses a custom Jackson
+ * serializer that writes the metrics collection as a raw JSON array, not as an object with a
+ * "metrics" field.
+ */
+ private static void overrideMetricCollectionSchema(final OpenAPI openApi) {
Review Comment:
Yup, you can see it's being generated as an object when response will be an array:
https://github.com/apache/flink/blob/release-2.2/docs/static/generated/rest_v1_dispatcher.yml#L1620-L1621
--
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
Re: [PR] [FLINK-39422] [REST][docs] Fix OpenAPI spec for `SlotSharingGroupId`, `MetricCollectionResponseBody`, and `SerializedThrowable` custom serializers [flink]
Posted by "nikhilsu (via GitHub)" <gi...@apache.org>.
nikhilsu commented on code in PR #27903:
URL: https://github.com/apache/flink/pull/27903#discussion_r3084166166
##########
flink-runtime/src/main/java/org/apache/flink/runtime/rest/messages/job/metrics/MetricCollectionResponseBody.java:
##########
@@ -46,61 +37,26 @@
* <pre>{@code
* [{"id": "metricName", "value": "1"}]
* }</pre>
- *
- * @see Serializer
- * @see Deserializer
- * @see org.apache.flink.runtime.rest.handler.legacy.metrics.MetricStore
*/
-@JsonSerialize(using = MetricCollectionResponseBody.Serializer.class)
-@JsonDeserialize(using = MetricCollectionResponseBody.Deserializer.class)
-public final class MetricCollectionResponseBody implements ResponseBody {
+public final class MetricCollectionResponseBody extends ArrayList<Metric> implements ResponseBody {
+
+ private static final long serialVersionUID = -1170348873871206965L;
- private final Collection<Metric> metrics;
+ // a default constructor is required for collection type marshaling
+ public MetricCollectionResponseBody() {}
public MetricCollectionResponseBody(Collection<Metric> metrics) {
- this.metrics = requireNonNull(metrics, "metrics must not be null");
+ super(requireNonNull(metrics, "metrics must not be null"));
}
+ @JsonIgnore
public Collection<Metric> getMetrics() {
- return metrics;
- }
-
- /** JSON serializer for {@link MetricCollectionResponseBody}. */
- public static class Serializer extends StdSerializer<MetricCollectionResponseBody> {
-
- private static final long serialVersionUID = 1L;
-
- protected Serializer() {
- super(MetricCollectionResponseBody.class);
- }
-
- @Override
- public void serialize(
- MetricCollectionResponseBody metricCollectionResponseBody,
- JsonGenerator jsonGenerator,
- SerializerProvider serializerProvider)
- throws IOException {
-
- jsonGenerator.writeObject(metricCollectionResponseBody.getMetrics());
- }
+ return this;
Review Comment:
Ok sounds good.
@RocMarshal IMHO we should return `this`. Returning a copy would be a behavior change - the old implementation returned `this.metrics` which was the same collection reference passed to the constructor, not a defensive copy. Callers that mutated the result would have mutated the internal state before too.
More importantly, the class itself is an `ArrayList` now. Returning a copy from getMetrics() while the object itself is mutable via add()/remove() would be inconsistent - you'd have two ways to access the same data with different mutability guarantees.
--
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