[Jobs] [Doc] Add Jobs openapi spec and REST API documentation #30417

architkulkarni · 2022-11-17T16:53:31Z

Why are these changes needed?

Adds an OpenAPI spec for the Jobs REST API
Adds an API doc page using Redoc.

The spec was generated via a combination of pydantic automatic schema generation, github copilot, and manual entry, but this won't be maintainable in the long term. When we move the dashboard server to FastAPI, this spec can be automatically generated.

Link to the doc page built from this PR: https://ray--30417.org.readthedocs.build/en/30417/cluster/running-applications/job-submission/rest.html

Related issue number

Checks

I've signed off every commit(by using the -s flag, i.e., git commit -s) in this PR.
I've run scripts/format.sh to lint the changes in this PR.
I've included any doc changes needed for https://docs.ray.io/en/master/.
I've made sure the tests are passing. Note that there might be a few flaky tests, see the recent failures at https://flakey-tests.ray.io/
Testing Strategy
- Unit tests
- Release tests
- This PR is not tested :(

Signed-off-by: Archit Kulkarni <[email protected]>

architkulkarni · 2022-11-17T18:37:15Z

doc/source/cluster/running-applications/job-submission/openapi.yml

+openapi: 3.1.0
+info:
+  title: Ray Jobs API
+  version: 4.0.0


The /api/version returns "4", not "4.0.0". Do these need to be kept in sync?

architkulkarni · 2022-11-17T18:37:58Z

doc/requirements-doc.txt

@@ -59,6 +59,7 @@ sphinxcontrib.yt==0.2.2
 sphinx-sitemap==2.2.0
 sphinx-thebe==0.1.1
 autodoc_pydantic==1.6.1
+sphinxcontrib-redoc==1.6.0


I don't have a particular reason to pin the version here, just following what's been done for other sphinx extensions

edoakes · 2022-11-17T22:28:14Z

@architkulkarni re: the process to generate the documentation, at a minimum we need to document how to do this in case others make changes in the future. Could you add a short README somewhere in the code?

edoakes · 2022-11-17T22:28:45Z

This is fantastic though!

Signed-off-by: Archit Kulkarni <[email protected]>

architkulkarni · 2022-11-17T22:41:41Z

@edoakes Good point, I added a note to job_head.py

edoakes · 2022-11-18T16:06:09Z

dashboard/modules/job/job_head.py

+    NOTE(architkulkarni): Please keep this class in sync with the OpenAPI spec at
+    `doc/source/cluster/running-applications/job-submission/openapi.yml`.
+    We currently do not automatically check that the OpenAPI
+    spec is in sync with the implementation.


I wouldn't know how to do this from the description alone, can you include instructions?

@edoakes updated, let me know if any parts are unclear

Signed-off-by: Archit Kulkarni <[email protected]>

…-openapi

DmitriGekhtman

stamp

edoakes

Suggest to get a review from @alanwguo before merging

alanwguo · 2022-11-21T18:42:39Z

doc/source/cluster/running-applications/job-submission/openapi.yml

@@ -0,0 +1,471 @@
+openapi: 3.1.0


is this file manually written? Did we consider switching the jobs api to use fast-api which has the ability to automatically generate openapi spec from the route definitions?

@rkooo567 and I have been chatting about doing a fast-api migration in the next 6 months so that we can have openapi specs for all of our APIs. If we are to do that, there's a chance that this spec will be slightly modified because it's generated instead of manually written. Will that be disruptive to users?

@alanwguo Yeah, migrating Jobs to FastAPI makes sense as the long term plan, we just didn't have time to do it for Ray 2.2. I think by marking the OpenAPI section of the doc as "Beta" as it currently is marked this PR, it should be okay if there are slight modifications in the future. The underlying API is stable and once we migrate to FastAPI we can mark the OpenAPI doc as "stable" as well.

dmatrix

minor nits. You can incorporate them if they make sense or add clarity. LGTM

dmatrix · 2022-11-21T21:27:52Z

dashboard/modules/job/job_head.py

+    paths in the @route decorators or in the Responses returned by the
+    methods (or any nested fields in the Responses), you will need to find the
+    corresponding field of the OpenAPI yaml file and update it manually, and
+    bump the version number in the yaml file and in this class's `get_version`.


The sentence is long. I would break it.

"update it manually." Also, bump the version number in the yaml file and in this class's get_version.

dmatrix · 2022-11-21T21:30:36Z

doc/source/cluster/running-applications/job-submission/openapi.yml

+      summary: |
+        Get Version
+      description: |
+        Get the Ray Jobs API version and the Ray version running on the cluster.


Should we use possessive here? Ray Jobs' API? If so then change in all description below.
I guess this would be tantamount to asking "Get Python's version" or "Get Python version."

I think either would work. But a grammar pedantic would suggest otherwise.

dmatrix · 2022-11-21T21:34:41Z

doc/source/cluster/running-applications/job-submission/openapi.yml

+                type: object
+                properties:
+                  version:
+                    description: The version of the Ray Jobs API running on the server.


Which one reads declaratively:
"The version of the Ray Jobs API running on the server." or
"Ray Jobs' API version running on the server."

dmatrix · 2022-11-21T21:41:58Z

doc/source/cluster/running-applications/job-submission/openapi.yml

+        - name: submission_id
+          in: path
+          description: |
+            The ID of the job to tail the logs of.


Ending with a proposition could read awkward in some cases. Would this be simpler: "Tail job ID's logs."

dmatrix · 2022-11-21T21:43:00Z

doc/source/cluster/running-applications/job-submission/openapi.yml

+          title: Job Id
+          description: >-
+            The job id. An id that is created for every job that is launched in ray.
+            This can be used to fetch data about jobs using ray core apis.


launched in Ray.

dmatrix · 2022-11-21T21:43:33Z

doc/source/cluster/running-applications/job-submission/openapi.yml

+          title: Job Id
+          description: >-
+            The job id. An id that is created for every job that is launched in ray.
+            This can be used to fetch data about jobs using ray core apis.


This can --> This id can be used to ...

dmatrix · 2022-11-21T21:46:07Z

doc/source/cluster/running-applications/job-submission/openapi.yml

+          title: Submission Id
+          description: >-
+            A submission id is an id created for every submission job. It can be used
+            to fetch data about jobs using the job submission apis.


The antecedent is ambiguous here. Does it refer to id or job. It normally refers to the last antecedent, which is job, and that is incorrect.

Suggest use "This id can be used to fetch ...

architkulkarni · 2022-11-21T23:42:21Z

Thanks @dmatrix, I will incorporate these in a follow-up PR!

…oject#30417) Adds an OpenAPI spec for the Jobs REST API Adds an API doc page using Redoc. Signed-off-by: Weichen Xu <[email protected]>

architkulkarni added 5 commits November 17, 2022 01:53

Add OpenAPI doc for Jobs REST API

24dbb9a

Signed-off-by: Archit Kulkarni <[email protected]>

Fix delete return type

9430a72

Signed-off-by: Archit Kulkarni <[email protected]>

Fix sdk link

1cdfd0a

Signed-off-by: Archit Kulkarni <[email protected]>

Improve error for delete

d903a27

Signed-off-by: Archit Kulkarni <[email protected]>

Change job_id to submission_id

5f99e59

Signed-off-by: Archit Kulkarni <[email protected]>

architkulkarni requested a review from wuisawesome as a code owner November 17, 2022 16:53

architkulkarni assigned alanwguo Nov 17, 2022

architkulkarni requested review from DmitriGekhtman, maxpumperla, pcmoritz and a team as code owners November 17, 2022 16:53

architkulkarni assigned edoakes and simon-mo Nov 17, 2022

architkulkarni commented Nov 17, 2022

View reviewed changes

architkulkarni added 2 commits November 17, 2022 14:41

Add README to job_head

1218632

Signed-off-by: Archit Kulkarni <[email protected]>

Lint

402a6ab

Signed-off-by: Archit Kulkarni <[email protected]>

Remove stray import

7c6c596

edoakes reviewed Nov 18, 2022

View reviewed changes

architkulkarni added 2 commits November 18, 2022 10:52

Improve instructions

a822fc9

Signed-off-by: Archit Kulkarni <[email protected]>

Merge branch 'master' of https://github.com/ray-project/ray into jobs…

f30f495

…-openapi

DmitriGekhtman approved these changes Nov 18, 2022

View reviewed changes

edoakes approved these changes Nov 18, 2022

View reviewed changes

architkulkarni added the Ray 2.2 label Nov 19, 2022

alanwguo reviewed Nov 21, 2022

View reviewed changes

dmatrix approved these changes Nov 21, 2022

View reviewed changes

architkulkarni merged commit 4e39367 into ray-project:master Nov 21, 2022

architkulkarni mentioned this pull request Dec 8, 2022

[Doc] [Jobs] Address comments on Jobs OpenAPI description #30975

Merged

7 tasks

architkulkarni mentioned this pull request Jan 23, 2023

[Docs] [Job] Tail Job logs REST endpoint missing documentation #25512

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[Jobs] [Doc] Add Jobs openapi spec and REST API documentation #30417

[Jobs] [Doc] Add Jobs openapi spec and REST API documentation #30417

architkulkarni commented Nov 17, 2022 •

edited

Loading

architkulkarni Nov 17, 2022

architkulkarni Nov 17, 2022

edoakes commented Nov 17, 2022

edoakes commented Nov 17, 2022

architkulkarni commented Nov 17, 2022

edoakes Nov 18, 2022

architkulkarni Nov 18, 2022

DmitriGekhtman left a comment

edoakes left a comment

alanwguo Nov 21, 2022

architkulkarni Nov 21, 2022 •

edited

Loading

dmatrix left a comment

dmatrix Nov 21, 2022

dmatrix Nov 21, 2022

dmatrix Nov 21, 2022

dmatrix Nov 21, 2022

dmatrix Nov 21, 2022

dmatrix Nov 21, 2022

dmatrix Nov 21, 2022

architkulkarni commented Nov 21, 2022

[Jobs] [Doc] Add Jobs openapi spec and REST API documentation #30417

[Jobs] [Doc] Add Jobs openapi spec and REST API documentation #30417

Conversation

architkulkarni commented Nov 17, 2022 • edited Loading

Why are these changes needed?

Related issue number

Checks

Choose a reason for hiding this comment

Choose a reason for hiding this comment

edoakes commented Nov 17, 2022

edoakes commented Nov 17, 2022

architkulkarni commented Nov 17, 2022

Choose a reason for hiding this comment

Choose a reason for hiding this comment

DmitriGekhtman left a comment

Choose a reason for hiding this comment

edoakes left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

architkulkarni Nov 21, 2022 • edited Loading

Choose a reason for hiding this comment

dmatrix left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

architkulkarni commented Nov 21, 2022

architkulkarni commented Nov 17, 2022 •

edited

Loading

architkulkarni Nov 21, 2022 •

edited

Loading