This is an automated email from the ASF dual-hosted git repository.
ethanfeng pushed a commit to branch main
in repository https://gitbox.apache.org/repos/asf/celeborn.git
The following commit(s) were added to refs/heads/main by this push:
new 1596cffb3 [CELEBORN-1607] Enable `useEnumCaseInsensitive` for
openapi-generator
1596cffb3 is described below
commit 1596cffb39d69141fbdcb854339af64f96925880
Author: Wang, Fei <[email protected]>
AuthorDate: Mon Sep 23 20:43:04 2024 +0800
[CELEBORN-1607] Enable `useEnumCaseInsensitive` for openapi-generator
### What changes were proposed in this pull request?
Enable `useEnumCaseInsensitive` for openapi-generator.
And then in celeborn server end, the enum will be mapped to celeborn
internal WorkerEventType.
### Why are the changes needed?
I met exception when sending worker event with openapi sdk.
```
Exception in thread "main" ApiException{code=400,
responseHeaders={Server=[Jetty(9.4.52.v20230823)], Content-Length=[491],
Date=[Fri, 20 Sep 2024 23:50:27 GMT], Content-Type=[text/plain]},
responseBody='Cannot deserialize value of type
`org.apache.celeborn.rest.v1.model.SendWorkerEventRequest$EventTypeEnum` from
String "DecommissionThenIdle": not one of the values accepted for Enum class:
[DECOMMISSION_THEN_IDLE, GRACEFUL, NONE, DECOMMISSION, IMMEDIATELY,
RECOMMISSION]
at [Source:
(org.glassfish.jersey.message.internal.ReaderInterceptorExecutor$UnCloseableInputStream);
line: 1, column: 14] (through reference chain:
org.apache.celeborn.rest.v1.model.SendWorkerEventRequest["eventType"])'}
at
org.apache.celeborn.rest.v1.master.invoker.ApiClient.processResponse(ApiClient.java:913)
at
org.apache.celeborn.rest.v1.master.invoker.ApiClient.invokeAPI(ApiClient.java:1000)
at
org.apache.celeborn.rest.v1.master.WorkerApi.sendWorkerEvent(WorkerApi.java:378)
at
org.apache.celeborn.rest.v1.master.WorkerApi.sendWorkerEvent(WorkerApi.java:334)
at org.example.Main.main(Main.java:22)
```
The testing code to re-produce:
```
package org.example;
import org.apache.celeborn.rest.v1.master.WorkerApi;
import org.apache.celeborn.rest.v1.master.invoker.ApiClient;
import org.apache.celeborn.rest.v1.model.ExcludeWorkerRequest;
import org.apache.celeborn.rest.v1.model.SendWorkerEventRequest;
import org.apache.celeborn.rest.v1.model.WorkerId;
public class Main {
public static void main(String[] args) throws Exception {
String cmUrl = "http://localhost:9098";
WorkerApi workerApi = new WorkerApi(new
ApiClient().setBasePath(cmUrl));
workerApi.excludeWorker(new ExcludeWorkerRequest()
.addAddItem(new WorkerId()
.host("localhost")
.rpcPort(1)
.pushPort(2)
.fetchPort(3)
.replicatePort(4)));
workerApi.sendWorkerEvent(new SendWorkerEventRequest()
.addWorkersItem(new WorkerId()
.host("127.0.0.1")
.rpcPort(56116)
.pushPort(56117)
.fetchPort(56119)
.replicatePort(56118))
.eventType(SendWorkerEventRequest.EventTypeEnum.DECOMMISSION_THEN_IDLE));
}
}
```
Seems because for the EventTypeEnum, the name and value not the same and
then cause this issue.
Not sure why the UT passed, but the integration testing failed.
For EventTypeEnum, because its value is case sensitive, so we meet this
issue.
https://github.com/apache/celeborn/blob/8734d1663884345477257d2f10f4c19732c6f1c3/openapi/openapi-client/src/main/java/org/apache/celeborn/rest/v1/model/SendWorkerEventRequest.java#L47-L83
Related issue in jersey end I think,
https://github.com/eclipse-ee4j/jersey/issues/5288
In this PR, `useEnumCaseInsensitive` is enabled for openapi-generator.
### Does this PR introduce _any_ user-facing change?
No, there is not user facing change and this SDK has not been released yet.
### How was this patch tested?
Existing UT and Integration testing.
<img width="1265" alt="image"
src="https://github.com/user-attachments/assets/6a34a0dd-c474-4e8d-b372-19b0fda94972">
Closes #2754 from turboFei/eventTypeEnumMapping.
Authored-by: Wang, Fei <[email protected]>
Signed-off-by: mingji <[email protected]>
---
.../common/protocol/message/ControlMessages.scala | 4 ++--
.../apache/celeborn/service/deploy/master/Master.scala | 2 +-
.../deploy/master/http/api/ApiMasterResource.scala | 5 ++++-
.../deploy/master/http/api/v1/WorkerResource.scala | 17 ++++++++++++++++-
.../master/http/api/v1/ApiV1MasterResourceSuite.scala | 2 +-
openapi/openapi-client/pom.xml | 2 ++
.../apache/celeborn/rest/v1/model/DynamicConfig.java | 2 +-
.../celeborn/rest/v1/model/PartitionLocationData.java | 4 ++--
.../celeborn/rest/v1/model/SendWorkerEventRequest.java | 14 +++++++-------
.../celeborn/rest/v1/model/WorkerExitRequest.java | 10 +++++-----
.../src/main/openapi3/master_rest_v1.yaml | 12 ++++++------
.../src/main/openapi3/worker_rest_v1.yaml | 10 +++++-----
project/CelebornBuild.scala | 1 +
.../org/apache/celeborn/server/common/HttpService.scala | 6 ++++--
.../worker/http/api/v1/ApiV1OpenapiClientSuite.scala | 2 +-
15 files changed, 58 insertions(+), 35 deletions(-)
diff --git
a/common/src/main/scala/org/apache/celeborn/common/protocol/message/ControlMessages.scala
b/common/src/main/scala/org/apache/celeborn/common/protocol/message/ControlMessages.scala
index c2c7b6bf4..e68212baf 100644
---
a/common/src/main/scala/org/apache/celeborn/common/protocol/message/ControlMessages.scala
+++
b/common/src/main/scala/org/apache/celeborn/common/protocol/message/ControlMessages.scala
@@ -425,11 +425,11 @@ object ControlMessages extends Logging {
object WorkerEventRequest {
def apply(
workers: util.List[WorkerInfo],
- eventType: String,
+ eventType: WorkerEventType,
requestId: String): PbWorkerEventRequest =
PbWorkerEventRequest.newBuilder()
.setRequestId(requestId)
- .setWorkerEventType(WorkerEventType.valueOf(eventType))
+ .setWorkerEventType(eventType)
.addAllWorkers(workers.asScala.map { workerInfo =>
PbSerDeUtils.toPbWorkerInfo(workerInfo, true, false)
}.toList.asJava)
diff --git
a/master/src/main/scala/org/apache/celeborn/service/deploy/master/Master.scala
b/master/src/main/scala/org/apache/celeborn/service/deploy/master/Master.scala
index 4c65e9816..7d8e5cc2d 100644
---
a/master/src/main/scala/org/apache/celeborn/service/deploy/master/Master.scala
+++
b/master/src/main/scala/org/apache/celeborn/service/deploy/master/Master.scala
@@ -1218,7 +1218,7 @@ private[celeborn] class Master(
}
override def handleWorkerEvent(
- workerEventType: String,
+ workerEventType: WorkerEventType,
workers: Seq[WorkerInfo]): HandleResponse = {
val sb = new StringBuilder()
try {
diff --git
a/master/src/main/scala/org/apache/celeborn/service/deploy/master/http/api/ApiMasterResource.scala
b/master/src/main/scala/org/apache/celeborn/service/deploy/master/http/api/ApiMasterResource.scala
index ee6fcdd67..ac326c5fb 100644
---
a/master/src/main/scala/org/apache/celeborn/service/deploy/master/http/api/ApiMasterResource.scala
+++
b/master/src/main/scala/org/apache/celeborn/service/deploy/master/http/api/ApiMasterResource.scala
@@ -26,6 +26,7 @@ import io.swagger.v3.oas.annotations.tags.Tag
import org.apache.commons.lang3.StringUtils
import org.apache.celeborn.common.meta.WorkerInfo
+import org.apache.celeborn.common.protocol.WorkerEventType
import org.apache.celeborn.server.common.http.api.ApiRequestContext
@Tag(name = "Deprecated")
@@ -136,7 +137,9 @@ class ApiMasterResource extends ApiRequestContext {
}
sb.append("============================ Handle Worker Event
=============================\n")
val workerList =
workers.split(",").filter(_.nonEmpty).map(WorkerInfo.fromUniqueId)
- sb.append(httpService.handleWorkerEvent(normalizeParam(eventType),
workerList)._2)
+ sb.append(httpService.handleWorkerEvent(
+ WorkerEventType.valueOf(normalizeParam(eventType)),
+ workerList)._2)
sb.toString()
}
}
diff --git
a/master/src/main/scala/org/apache/celeborn/service/deploy/master/http/api/v1/WorkerResource.scala
b/master/src/main/scala/org/apache/celeborn/service/deploy/master/http/api/v1/WorkerResource.scala
index a41a07c5d..8d1b20bc3 100644
---
a/master/src/main/scala/org/apache/celeborn/service/deploy/master/http/api/v1/WorkerResource.scala
+++
b/master/src/main/scala/org/apache/celeborn/service/deploy/master/http/api/v1/WorkerResource.scala
@@ -26,7 +26,9 @@ import io.swagger.v3.oas.annotations.media.{Content, Schema}
import io.swagger.v3.oas.annotations.responses.ApiResponse
import io.swagger.v3.oas.annotations.tags.Tag
+import org.apache.celeborn.common.protocol.WorkerEventType
import org.apache.celeborn.rest.v1.model._
+import org.apache.celeborn.rest.v1.model.SendWorkerEventRequest.EventTypeEnum
import org.apache.celeborn.server.common.http.api.ApiRequestContext
import org.apache.celeborn.server.common.http.api.v1.ApiUtils
import org.apache.celeborn.service.deploy.master.Master
@@ -137,7 +139,8 @@ class WorkerResource extends ApiRequestContext {
throw new BadRequestException(
s"None of the workers are known:
${unknownWorkers.map(_.readableAddress).mkString(", ")}")
}
- val (success, msg) =
httpService.handleWorkerEvent(request.getEventType.toString, workers)
+ val (success, msg) =
+ httpService.handleWorkerEvent(toWorkerEventType(request.getEventType),
workers)
val finalMsg =
if (unknownWorkers.isEmpty) {
msg
@@ -146,4 +149,16 @@ class WorkerResource extends ApiRequestContext {
}
new HandleResponse().success(success).message(finalMsg)
}
+
+ private def toWorkerEventType(enum: EventTypeEnum): WorkerEventType = {
+ enum match {
+ case EventTypeEnum.NONE => WorkerEventType.None
+ case EventTypeEnum.IMMEDIATELY => WorkerEventType.Immediately
+ case EventTypeEnum.DECOMMISSION => WorkerEventType.Decommission
+ case EventTypeEnum.DECOMMISSIONTHENIDLE =>
WorkerEventType.DecommissionThenIdle
+ case EventTypeEnum.GRACEFUL => WorkerEventType.Graceful
+ case EventTypeEnum.RECOMMISSION => WorkerEventType.Recommission
+ case _ => WorkerEventType.UNRECOGNIZED
+ }
+ }
}
diff --git
a/master/src/test/scala/org/apache/celeborn/service/deploy/master/http/api/v1/ApiV1MasterResourceSuite.scala
b/master/src/test/scala/org/apache/celeborn/service/deploy/master/http/api/v1/ApiV1MasterResourceSuite.scala
index 35649ec75..5ae688d91 100644
---
a/master/src/test/scala/org/apache/celeborn/service/deploy/master/http/api/v1/ApiV1MasterResourceSuite.scala
+++
b/master/src/test/scala/org/apache/celeborn/service/deploy/master/http/api/v1/ApiV1MasterResourceSuite.scala
@@ -143,7 +143,7 @@ class ApiV1MasterResourceSuite extends
ApiV1BaseResourceSuite {
Entity.entity(sendWorkerEventRequest, MediaType.APPLICATION_JSON))
assert(HttpServletResponse.SC_BAD_REQUEST == response.getStatus)
assert(
- response.readEntity(classOf[String]).contains("eventType(None) and
workers([]) are required"))
+ response.readEntity(classOf[String]).contains("eventType(NONE) and
workers([]) are required"))
sendWorkerEventRequest.workers(Collections.singletonList(worker))
response =
webTarget.path("workers/events").request(MediaType.APPLICATION_JSON).post(
Entity.entity(sendWorkerEventRequest, MediaType.APPLICATION_JSON))
diff --git a/openapi/openapi-client/pom.xml b/openapi/openapi-client/pom.xml
index e608d3e30..7309cb8a2 100644
--- a/openapi/openapi-client/pom.xml
+++ b/openapi/openapi-client/pom.xml
@@ -244,6 +244,7 @@
<hideGenerationTimestamp>true</hideGenerationTimestamp>
<supportUrlQuery>false</supportUrlQuery>
<annotationLibrary>none</annotationLibrary>
+ <useEnumCaseInsensitive>true</useEnumCaseInsensitive>
</configOptions>
</configuration>
</execution>
@@ -278,6 +279,7 @@
<hideGenerationTimestamp>true</hideGenerationTimestamp>
<supportUrlQuery>false</supportUrlQuery>
<annotationLibrary>none</annotationLibrary>
+ <useEnumCaseInsensitive>true</useEnumCaseInsensitive>
</configOptions>
</configuration>
</execution>
diff --git
a/openapi/openapi-client/src/main/java/org/apache/celeborn/rest/v1/model/DynamicConfig.java
b/openapi/openapi-client/src/main/java/org/apache/celeborn/rest/v1/model/DynamicConfig.java
index c0ae71fb6..23d30a1e2 100644
---
a/openapi/openapi-client/src/main/java/org/apache/celeborn/rest/v1/model/DynamicConfig.java
+++
b/openapi/openapi-client/src/main/java/org/apache/celeborn/rest/v1/model/DynamicConfig.java
@@ -71,7 +71,7 @@ public class DynamicConfig {
@JsonCreator
public static LevelEnum fromValue(String value) {
for (LevelEnum b : LevelEnum.values()) {
- if (b.value.equals(value)) {
+ if (b.value.equalsIgnoreCase(value)) {
return b;
}
}
diff --git
a/openapi/openapi-client/src/main/java/org/apache/celeborn/rest/v1/model/PartitionLocationData.java
b/openapi/openapi-client/src/main/java/org/apache/celeborn/rest/v1/model/PartitionLocationData.java
index 55311f03b..dd2cd4577 100644
---
a/openapi/openapi-client/src/main/java/org/apache/celeborn/rest/v1/model/PartitionLocationData.java
+++
b/openapi/openapi-client/src/main/java/org/apache/celeborn/rest/v1/model/PartitionLocationData.java
@@ -74,7 +74,7 @@ public class PartitionLocationData {
@JsonCreator
public static ModeEnum fromValue(String value) {
for (ModeEnum b : ModeEnum.values()) {
- if (b.value.equals(value)) {
+ if (b.value.equalsIgnoreCase(value)) {
return b;
}
}
@@ -123,7 +123,7 @@ public class PartitionLocationData {
@JsonCreator
public static StorageEnum fromValue(String value) {
for (StorageEnum b : StorageEnum.values()) {
- if (b.value.equals(value)) {
+ if (b.value.equalsIgnoreCase(value)) {
return b;
}
}
diff --git
a/openapi/openapi-client/src/main/java/org/apache/celeborn/rest/v1/model/SendWorkerEventRequest.java
b/openapi/openapi-client/src/main/java/org/apache/celeborn/rest/v1/model/SendWorkerEventRequest.java
index c0934c840..290b9ad4e 100644
---
a/openapi/openapi-client/src/main/java/org/apache/celeborn/rest/v1/model/SendWorkerEventRequest.java
+++
b/openapi/openapi-client/src/main/java/org/apache/celeborn/rest/v1/model/SendWorkerEventRequest.java
@@ -45,17 +45,17 @@ public class SendWorkerEventRequest {
* The type of the event.
*/
public enum EventTypeEnum {
- IMMEDIATELY("Immediately"),
+ IMMEDIATELY("IMMEDIATELY"),
- DECOMMISSION("Decommission"),
+ DECOMMISSION("DECOMMISSION"),
- DECOMMISSION_THEN_IDLE("DecommissionThenIdle"),
+ DECOMMISSIONTHENIDLE("DECOMMISSIONTHENIDLE"),
- GRACEFUL("Graceful"),
+ GRACEFUL("GRACEFUL"),
- RECOMMISSION("Recommission"),
+ RECOMMISSION("RECOMMISSION"),
- NONE("None");
+ NONE("NONE");
private String value;
@@ -76,7 +76,7 @@ public class SendWorkerEventRequest {
@JsonCreator
public static EventTypeEnum fromValue(String value) {
for (EventTypeEnum b : EventTypeEnum.values()) {
- if (b.value.equals(value)) {
+ if (b.value.equalsIgnoreCase(value)) {
return b;
}
}
diff --git
a/openapi/openapi-client/src/main/java/org/apache/celeborn/rest/v1/model/WorkerExitRequest.java
b/openapi/openapi-client/src/main/java/org/apache/celeborn/rest/v1/model/WorkerExitRequest.java
index e6bf683a7..79fb12d03 100644
---
a/openapi/openapi-client/src/main/java/org/apache/celeborn/rest/v1/model/WorkerExitRequest.java
+++
b/openapi/openapi-client/src/main/java/org/apache/celeborn/rest/v1/model/WorkerExitRequest.java
@@ -40,13 +40,13 @@ public class WorkerExitRequest {
* The type of the worker exit request.
*/
public enum TypeEnum {
- DECOMMISSION("Decommission"),
+ DECOMMISSION("DECOMMISSION"),
- GRACEFUL("Graceful"),
+ GRACEFUL("GRACEFUL"),
- IMMEDIATELY("Immediately"),
+ IMMEDIATELY("IMMEDIATELY"),
- NONE("None");
+ NONE("NONE");
private String value;
@@ -67,7 +67,7 @@ public class WorkerExitRequest {
@JsonCreator
public static TypeEnum fromValue(String value) {
for (TypeEnum b : TypeEnum.values()) {
- if (b.value.equals(value)) {
+ if (b.value.equalsIgnoreCase(value)) {
return b;
}
}
diff --git a/openapi/openapi-client/src/main/openapi3/master_rest_v1.yaml
b/openapi/openapi-client/src/main/openapi3/master_rest_v1.yaml
index c03fe11eb..0c4948abf 100644
--- a/openapi/openapi-client/src/main/openapi3/master_rest_v1.yaml
+++ b/openapi/openapi-client/src/main/openapi3/master_rest_v1.yaml
@@ -706,12 +706,12 @@ components:
type: string
description: The type of the event.
enum:
- - Immediately
- - Decommission
- - DecommissionThenIdle
- - Graceful
- - Recommission
- - None
+ - IMMEDIATELY
+ - DECOMMISSION
+ - DECOMMISSIONTHENIDLE
+ - GRACEFUL
+ - RECOMMISSION
+ - NONE
workers:
type: array
description: The workers to send the event.
diff --git a/openapi/openapi-client/src/main/openapi3/worker_rest_v1.yaml
b/openapi/openapi-client/src/main/openapi3/worker_rest_v1.yaml
index 3db792a54..29bd2865d 100644
--- a/openapi/openapi-client/src/main/openapi3/worker_rest_v1.yaml
+++ b/openapi/openapi-client/src/main/openapi3/worker_rest_v1.yaml
@@ -602,13 +602,13 @@ components:
properties:
type:
type: string
- default: None
+ default: NONE
description: The type of the worker exit request.
enum:
- - Decommission
- - Graceful
- - Immediately
- - None
+ - DECOMMISSION
+ - GRACEFUL
+ - IMMEDIATELY
+ - NONE
HandleResponse:
type: object
diff --git a/project/CelebornBuild.scala b/project/CelebornBuild.scala
index 25f904b81..1c18aab0d 100644
--- a/project/CelebornBuild.scala
+++ b/project/CelebornBuild.scala
@@ -1364,6 +1364,7 @@ object CelebornOpenApi {
"supportUrlQuery" -> "false",
"annotationLibrary" -> "none",
"templateDir" -> s"$openApiSpecDir/templates",
+ "useEnumCaseInsensitive" -> "true"
)
)
diff --git
a/service/src/main/scala/org/apache/celeborn/server/common/HttpService.scala
b/service/src/main/scala/org/apache/celeborn/server/common/HttpService.scala
index 2efcec545..2301ea25c 100644
--- a/service/src/main/scala/org/apache/celeborn/server/common/HttpService.scala
+++ b/service/src/main/scala/org/apache/celeborn/server/common/HttpService.scala
@@ -27,7 +27,7 @@ import org.eclipse.jetty.servlet.FilterHolder
import org.apache.celeborn.common.CelebornConf
import org.apache.celeborn.common.internal.Logging
import org.apache.celeborn.common.meta.WorkerInfo
-import org.apache.celeborn.common.protocol.TransportModuleConstants
+import org.apache.celeborn.common.protocol.{TransportModuleConstants,
WorkerEventType}
import org.apache.celeborn.common.util.Utils
import org.apache.celeborn.server.common.http.HttpServer
import org.apache.celeborn.server.common.http.api.ApiRootResource
@@ -187,7 +187,9 @@ abstract class HttpService extends Service with Logging {
def exit(exitType: String): String = throw new
UnsupportedOperationException()
- def handleWorkerEvent(workerEventType: String, workers: Seq[WorkerInfo]):
HandleResponse =
+ def handleWorkerEvent(
+ workerEventType: WorkerEventType,
+ workers: Seq[WorkerInfo]): HandleResponse =
throw new UnsupportedOperationException()
def getWorkerEventInfo(): String = throw new UnsupportedOperationException()
diff --git
a/worker/src/test/scala/org/apache/celeborn/service/deploy/worker/http/api/v1/ApiV1OpenapiClientSuite.scala
b/worker/src/test/scala/org/apache/celeborn/service/deploy/worker/http/api/v1/ApiV1OpenapiClientSuite.scala
index e5368cf6d..1557f388a 100644
---
a/worker/src/test/scala/org/apache/celeborn/service/deploy/worker/http/api/v1/ApiV1OpenapiClientSuite.scala
+++
b/worker/src/test/scala/org/apache/celeborn/service/deploy/worker/http/api/v1/ApiV1OpenapiClientSuite.scala
@@ -108,7 +108,7 @@ class ApiV1OpenapiClientSuite extends
ApiV1WorkerOpenapiClientSuite {
handleResponse = api.sendWorkerEvent(
new SendWorkerEventRequest().addWorkersItem(workerId).eventType(
- EventTypeEnum.DECOMMISSION_THEN_IDLE))
+ EventTypeEnum.DECOMMISSIONTHENIDLE))
assert(handleResponse.getSuccess)
assert(!api.getWorkerEvents.getWorkerEvents.isEmpty)