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 12:53:53 UTC
[GitHub] [flink] zentol opened a new pull request, #22355: [FLINK-31733][docs] Fix/Detect OpenAPI model name clashes
zentol opened a new pull request, #22355:
URL: https://github.com/apache/flink/pull/22355
Fixes 5 cases where a name clash caused certain classes to not be documented properly. Name clashes were silently ignored by the generator.
The offending classes are now annotated to provide a unique name, and the generator was enhanced to detect such clashes.
--
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 #22355: [FLINK-31733][docs] Fix/Detect OpenAPI model name clashes
Posted by "zentol (via GitHub)" <gi...@apache.org>.
zentol commented on code in PR #22355:
URL: https://github.com/apache/flink/pull/22355#discussion_r1159705422
##########
flink-runtime/src/main/java/org/apache/flink/runtime/rest/messages/checkpoints/CheckpointingStatistics.java:
##########
@@ -198,6 +200,7 @@ public int hashCode() {
}
/** Checkpoint summary. */
+ @Schema(name = "CheckpointStatisticsSummary")
Review Comment:
That's what I tried first, but the result wasn't satisfying. `JobDetailsInfoJobVertexDetailsInfo` isn't a great name :/
--
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 #22355: [FLINK-31733][docs] Fix/Detect OpenAPI model name clashes
Posted by "dmvk (via GitHub)" <gi...@apache.org>.
dmvk commented on code in PR #22355:
URL: https://github.com/apache/flink/pull/22355#discussion_r1159700760
##########
flink-runtime/src/main/java/org/apache/flink/runtime/rest/messages/checkpoints/CheckpointingStatistics.java:
##########
@@ -198,6 +200,7 @@ public int hashCode() {
}
/** Checkpoint summary. */
+ @Schema(name = "CheckpointStatisticsSummary")
Review Comment:
It seems that there is mostly a problem with the nested classes. Can we simply always use the wrapping class as a "namespace"?
##########
flink-runtime/src/main/java/org/apache/flink/runtime/rest/messages/checkpoints/CheckpointingStatistics.java:
##########
@@ -198,6 +200,7 @@ public int hashCode() {
}
/** Checkpoint summary. */
+ @Schema(name = "CheckpointStatisticsSummary")
Review Comment:
It seems that there is mostly a problem with the nested classes. Can we simply always use the wrapping class as a "prefix"?
--
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 #22355: [FLINK-31733][docs] Fix/Detect OpenAPI model name clashes
Posted by "dmvk (via GitHub)" <gi...@apache.org>.
dmvk commented on code in PR #22355:
URL: https://github.com/apache/flink/pull/22355#discussion_r1159691047
##########
flink-runtime/src/main/java/org/apache/flink/runtime/rest/messages/job/JobDetailsInfo.java:
##########
@@ -259,6 +261,7 @@ public String getJsonPlan() {
// ---------------------------------------------------
/** Detailed information about a job vertex. */
+ @Schema(name = "JobDetailsVertexInfo")
public static final class JobVertexDetailsInfo {
Review Comment:
Why don't we just rename the class? do we need to keep any bw compatibility on this level?
--
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 #22355: [FLINK-31733][docs] Fix/Detect OpenAPI model name clashes
Posted by "zentol (via GitHub)" <gi...@apache.org>.
zentol commented on code in PR #22355:
URL: https://github.com/apache/flink/pull/22355#discussion_r1159694301
##########
flink-runtime/src/main/java/org/apache/flink/runtime/rest/messages/job/JobDetailsInfo.java:
##########
@@ -259,6 +261,7 @@ public String getJsonPlan() {
// ---------------------------------------------------
/** Detailed information about a job vertex. */
+ @Schema(name = "JobDetailsVertexInfo")
public static final class JobVertexDetailsInfo {
Review Comment:
Compatibility isn't a concern.
--
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 #22355: [FLINK-31733][docs] Fix/Detect OpenAPI model name clashes
Posted by "flinkbot (via GitHub)" <gi...@apache.org>.
flinkbot commented on PR #22355:
URL: https://github.com/apache/flink/pull/22355#issuecomment-1497445109
<!--
Meta data
{
"version" : 1,
"metaDataEntries" : [ {
"hash" : "a5fcc4ccd3dd82afc7e41e561135a70fde2168e7",
"status" : "UNKNOWN",
"url" : "TBD",
"triggerID" : "a5fcc4ccd3dd82afc7e41e561135a70fde2168e7",
"triggerType" : "PUSH"
} ]
}-->
## CI report:
* a5fcc4ccd3dd82afc7e41e561135a70fde2168e7 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] dmvk commented on a diff in pull request #22355: [FLINK-31733][docs] Fix/Detect OpenAPI model name clashes
Posted by "dmvk (via GitHub)" <gi...@apache.org>.
dmvk commented on code in PR #22355:
URL: https://github.com/apache/flink/pull/22355#discussion_r1159689747
##########
flink-docs/src/main/java/org/apache/flink/docs/rest/OpenApiSpecGenerator.java:
##########
@@ -494,4 +498,30 @@ private static PathItem.HttpMethod convert(HttpMethodWrapper wrapper) {
}
throw new IllegalArgumentException("not supported");
}
+
+ /** A {@link TypeNameResolver} that detects name-clashes between top-level and inner classes. */
+ public static class NameClashDetectingTypeNameResolver extends TypeNameResolver {
Review Comment:
This is nice. <3
--
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 #22355: [FLINK-31733][docs] Fix/Detect OpenAPI model name clashes
Posted by "zentol (via GitHub)" <gi...@apache.org>.
zentol commented on code in PR #22355:
URL: https://github.com/apache/flink/pull/22355#discussion_r1159705754
##########
flink-runtime/src/main/java/org/apache/flink/runtime/rest/messages/checkpoints/CheckpointingStatistics.java:
##########
@@ -198,6 +200,7 @@ public int hashCode() {
}
/** Checkpoint summary. */
+ @Schema(name = "CheckpointStatisticsSummary")
Review Comment:
It's possible to do though in the custom `TypeNameResolver`.
--
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 #22355: [FLINK-31733][docs] Fix/Detect OpenAPI model name clashes
Posted by "zentol (via GitHub)" <gi...@apache.org>.
zentol commented on code in PR #22355:
URL: https://github.com/apache/flink/pull/22355#discussion_r1159692620
##########
flink-runtime/src/main/java/org/apache/flink/runtime/rest/messages/job/JobDetailsInfo.java:
##########
@@ -259,6 +261,7 @@ public String getJsonPlan() {
// ---------------------------------------------------
/** Detailed information about a job vertex. */
+ @Schema(name = "JobDetailsVertexInfo")
public static final class JobVertexDetailsInfo {
Review Comment:
We could rename all classes, I just wasn't sure if we want to go down that route purely for documentation work.
--
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 #22355: [FLINK-31733][docs] Fix/Detect OpenAPI model name clashes
Posted by "zentol (via GitHub)" <gi...@apache.org>.
zentol commented on code in PR #22355:
URL: https://github.com/apache/flink/pull/22355#discussion_r1159697530
##########
flink-runtime/src/main/java/org/apache/flink/runtime/rest/messages/job/JobDetailsInfo.java:
##########
@@ -259,6 +261,7 @@ public String getJsonPlan() {
// ---------------------------------------------------
/** Detailed information about a job vertex. */
+ @Schema(name = "JobDetailsVertexInfo")
public static final class JobVertexDetailsInfo {
Review Comment:
There are of course advantages to renaming the class, like it being easier to track down the internal representation of something in the API. But I'm not sure if that's worth the hassle of restricting us to unique class names everywhere.
--
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 #22355: [FLINK-31733][docs] Fix/Detect OpenAPI model name clashes
Posted by "dmvk (via GitHub)" <gi...@apache.org>.
dmvk commented on code in PR #22355:
URL: https://github.com/apache/flink/pull/22355#discussion_r1159691854
##########
flink-runtime/src/test/java/org/apache/flink/runtime/rest/util/DocumentingRestEndpoint.java:
##########
@@ -50,4 +51,11 @@ List<Tuple2<RestHandlerSpecification, ChannelInboundHandler>> initializeHandlers
spec2.getTargetRestEndpointURL()))
.collect(Collectors.toList());
}
+
+ static DocumentingRestEndpoint forRestHandlerSpecifications(RestHandlerSpecification... specs) {
+ return localAddressFuture ->
+ Arrays.asList(specs).stream()
Review Comment:
```suggestion
Arrays.stream(specs)
```
--
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 #22355: [FLINK-31733][docs] Fix/Detect OpenAPI model name clashes
Posted by "zentol (via GitHub)" <gi...@apache.org>.
zentol merged PR #22355:
URL: https://github.com/apache/flink/pull/22355
--
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