-
Notifications
You must be signed in to change notification settings - Fork 271
Add Delegation Service Module and REST Framework #1981
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
base: main
Are you sure you want to change the base?
Conversation
cc: @eric-maynard, @flyrain |
Please allow some more review time. From my POV the dev list discussion does not have a conclusion yet 🤔 |
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 please wait with this. We should discuss first which approach the project takes. No decision on any of the proposals on the dev mailing list has been made yet.
@Path("/api/v1/tasks/execute") | ||
@Consumes(MediaType.APPLICATION_JSON) | ||
@Produces(MediaType.APPLICATION_JSON) | ||
public class DelegationApi { |
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.
Can we add @experimental
here ?
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 isn't a Polaris API, and within the context of the delegation service (itself experimental), this is not really an experimental API.
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.
My proposal was more to "flag" it experimental (from code perspective). If the "whole" service is experimental, I'm fine with that.
Maybe a couple of tests to illustrate how it works would be welcome.
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.
From reading the proposal, I think what will actually be experimental will be the RemoteTaskExecutor
within Polaris, but that's not implemented in this 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.
Thank you @jbonofre for your concern! As @eric-maynard has mentioned, the task executor for delegating to the remote service can be flagged as experimental.
In regards to illustrating how it would all work, an end-to-end test is in preparation.
The question is which approach two use. There are two proposals on the dev-ML, with no technical pro/con discussion and therefore no decision. As mentioned in other discussions, things do take time. The whole topic is too sensitive and needs to be handled carefully, considering all aspects. Having one proposal not receiving comments and weeks after another, different counter-proposal plus a concrete implementation is not great. |
This PR aims to set up the Delegation Service module for development, starting with the REST API framework.
Discussion thread: