Convert job_manager to be async #27123
Conversation
Updates jobs api Updates snapshot api Updates state api Increases jobs api version to 2 Signed-off-by: Alan Guo <[email protected]>
alanwguo
requested review from
wuisawesome,
ijrsvt,
edoakes and
architkulkarni
as code owners
|
nice! I will review this today |
| if not self._job_info_client: | ||
| self._job_info_client = JobInfoStorageClient( | ||
| self._dashboard_head.gcs_aio_client | ||
| ) |
There was a problem hiding this comment.
why do we lazily construct this? add comment for future reader
dashboard/modules/version.py
Outdated
| # Version 1 -> 2: - Renamed job_id to submission_id. | ||
| # - Changed list_jobs sdk/cli/api to return a list | ||
| # instead of a dictionary. | ||
| CURRENT_VERSION = "2" |
There was a problem hiding this comment.
is this intended? or an artifact of stacked PRs? please make this a separate PR either way.
There was a problem hiding this comment.
Intended but yes, an orthogonal change. Will separate.
There was a problem hiding this comment.
Thanks. LGTM to merge after this is resolved.
edoakes
added
the
@author-action-required
Signed-off-by: Alan Guo <[email protected]>
alanwguo
removed
the
@author-action-required
Signed-off-by: Alan Guo <[email protected]>
Signed-off-by: Alan Guo <[email protected]>
|
I will take a look at the PR tomorrow (please ping me if I dont') |
edoakes
added
the
@author-action-required
| self.JOB_DATA_KEY.format(job_id=job_id), | ||
| async def put_info(self, job_id: str, data: JobInfo): | ||
| await self._gcs_aio_client.internal_kv_put( | ||
| self.JOB_DATA_KEY.format(job_id=job_id).encode(), |
There was a problem hiding this comment.
Btw, do you know what happens if you don't specify encode ? Does it just fail?
There was a problem hiding this comment.
Little concerned the original _internal_kv_put has a runtime type conversion & checking, but it doesn't I know this PR handles that correctly, but just in case.
There was a problem hiding this comment.
yes, without encode, there is an error that gets thrown.
Signed-off-by: Alan Guo <[email protected]>
alanwguo
added
v2.0.0-pick
and removed
@author-action-required
| return { | ||
| job_id: job_info | ||
| for job_id, job_info in await asyncio.gather( | ||
| *[get_job_info(job_id) for job_id in job_ids] |
There was a problem hiding this comment.
Btw, this is not a blocker for the PR. But I wonder @iycheng do we have APIs to get a list of entities from the internal kv?
Signed-off-by: Alan Guo <[email protected]>
|
@rkooo567 @architkulkarni @edoakes , please merge! |
Updates jobs api Updates snapshot api Updates state api Increases jobs api version to 2 Signed-off-by: Alan Guo [email protected] Why are these changes needed? follow-up for #25902 (comment)
Updates jobs api Updates snapshot api Updates state api Increases jobs api version to 2 Signed-off-by: Alan Guo [email protected] Why are these changes needed? follow-up for #25902 (comment)
Updates jobs api Updates snapshot api Updates state api Increases jobs api version to 2 Signed-off-by: Alan Guo [email protected] Why are these changes needed? follow-up for ray-project#25902 (comment) Signed-off-by: Stefan van der Kleij <[email protected]>
Updates jobs api
Updates snapshot api
Updates state api
Increases jobs api version to 2
Signed-off-by: Alan Guo [email protected]
Why are these changes needed?
follow-up for #25902 (comment)
Related issue number
Checks
git commit -s) in this PR.scripts/format.shto lint the changes in this PR.