-
Notifications
You must be signed in to change notification settings - Fork 996
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
Add job management APIs #302
Comments
Thanks for this @zhilingc! It seems like there are three types of "gets":
Can I suggest that we start with a Job RPC and a ListJobs RPC instead of a single RPC that does both? It would mirror this structure from the Kubernetes API I know (1) and (2) have use cases, but does (3) have a use case? If it does, then we can add it to ListJobsRequest. I also think that we should consider updating feature retrieval to this structure since it more closely follows usage of clients. |
I would like to be consistent with our existing APIs as much as possible, which happen to overload a single I think looking up jobs by store/featureset is probably more useful to users than looking up jobs by name... since |
I am happy staying consistent for the time being. I don't want to derail this request with a debate on naming, but I think ListJobs expresses intent more clearly than GetJobs. In terms of functionality there, I think we are proposing the same thing. I do think we need a GetJob method by Id, but I guess that isn't as pressing for now so it can be punted. |
Also, can we use Job Id instead of name? |
I agree. Like i mentioned, I'm just gunning for internal consistency as much as possible.
we could add it as one of the possible fields of the GetJobsRequest, but it doesn't really spark joy haha.
|
I mean I would like to have an RPC that only returns a single item, or fails otherwise. The example above is not clean. The equivalent of |
We should add a job management to the core API for
For retrieval, we could possibly retrieve jobs based on the existing GetFeatureSets and GetStores filters:
Jobs should be aborted by name.
The text was updated successfully, but these errors were encountered: