-
Notifications
You must be signed in to change notification settings - Fork 5.6k
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
[Serve] Define Java Backend #16169
[Serve] Define Java Backend #16169
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This looks great! I think the next step here that we need to start converting majority of the data classes (POJO classes) into protobuf types.
import java.io.Serializable; | ||
import org.apache.commons.lang3.builder.ReflectionToStringBuilder; | ||
|
||
/** |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Let's make sure to convert this into protobuf in the next PR?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yes. I'll design some plans about next PR this week.
|
||
this.hostActor = hostActor; | ||
this.keyListeners = keyListeners; | ||
this.snapshotIds = new ConcurrentHashMap<>(); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
👍
I think there are several format and lint issues, Additionally, are these tests ran at all? Can you try add them to https://github.com/ray-project/ray/blob/master/java/test.sh |
@liuyang-my ping about adding the tests. Thanks! |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM
<dependency> | ||
<groupId>com.google.guava</groupId> | ||
<artifactId>guava</artifactId> | ||
<version>27.0.1-jre</version> |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Unfortunately, we had upgraded guava to 30.0 to fix CVEs. The PR is #16650
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@liuyang-my Can you also fix it?
Why are these changes needed?
We hope to take Ray Serve to AntGroup and reuse the ability of Ray Serve in Ant's Ray Serving. In Ant, a large part of business scenarios come from Java developers, so Ray Serve is required to support Java backend.
In the milestone "[serve] Support Java Backend", we will enable Ray Serve to support Java Backend. Detailed design is in doc :https://docs.google.com/document/d/1b9VCZsC2jVhWw_c_ifMu_ED3NxkARcOnU-OFqUYu-r4/edit#heading=h.b3vvz7iwnixn
This PR is about the definition of Java Backend.
Related issue number
#15835
Checks
scripts/format.sh
to lint the changes in this PR.