Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[CELEBORN-1607] Enable useEnumCaseInsensitive for openapi-generator #2754

Closed
wants to merge 5 commits into from

Conversation

turboFei
Copy link
Member

@turboFei turboFei commented Sep 21, 2024

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.

public enum EventTypeEnum {
IMMEDIATELY("Immediately"),
DECOMMISSION("Decommission"),
DECOMMISSION_THEN_IDLE("DecommissionThenIdle"),
GRACEFUL("Graceful"),
RECOMMISSION("Recommission"),
NONE("None");
private String value;
EventTypeEnum(String value) {
this.value = value;
}
@JsonValue
public String getValue() {
return value;
}
@Override
public String toString() {
return String.valueOf(value);
}
@JsonCreator
public static EventTypeEnum fromValue(String value) {
for (EventTypeEnum b : EventTypeEnum.values()) {
if (b.value.equals(value)) {
return b;
}
}
throw new IllegalArgumentException("Unexpected value '" + value + "'");

Related issue in jersey end I think, eclipse-ee4j/jersey#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.
image

@turboFei turboFei changed the title [CELEBORN-1607] Fix openapi-client send worker event issue [CELEBORN-1607] Enable useEnumCaseInsensitive for openapi client Sep 21, 2024
@turboFei turboFei changed the title [CELEBORN-1607] Enable useEnumCaseInsensitive for openapi client [CELEBORN-1607] Enable useEnumCaseInsensitive for openapi-generator Sep 21, 2024
@turboFei turboFei changed the title [CELEBORN-1607] Enable useEnumCaseInsensitive for openapi-generator [CELEBORN-1607] Enable useEnumCaseInsensitive for openapi-generator Sep 21, 2024
@turboFei
Copy link
Member Author

cc @pan3793 @cxzl25 @cfmcgrady

Copy link
Contributor

@FMX FMX left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM. Thanks. Merged into main(v0.6.0).

@FMX FMX closed this in 1596cff Sep 23, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants