List jobs per job type #576

javiermtorres · 2025-01-07T17:39:25Z

What's changing

A GET request can be issued to the /jobs/{JOB_TYPE}/ endpoint, where currently JOB_TYPE is either inference or evaluate, to select all jobs of the desired type. A GET request can still be issued to /jobs/{UUID}/ where UUID is a v4 UUID to retrieve the details of a single job.

Closes #380

How to test it

As tested in the integration tests, two jobs, one inference and one evaluation, can be started, and only one would be selected with the new API.

Additional notes for reviewers

N/A

I already...

Tested the changes in a working environment to ensure they work as expected
Added some tests for any new functionality
Updated the documentation (both comments in code and product documentation under /docs)
Checked if a (backend) DB migration step was required and included it if required

lumigator/python/mzai/backend/backend/api/routes/jobs.py

veekaybee · 2025-01-08T17:03:02Z

lumigator/python/mzai/backend/backend/api/routes/jobs.py

@@ -103,20 +105,57 @@ def list_jobs(
    )


-@router.get("/{job_id}")
-def get_job(service: JobServiceDep, job_id: UUID) -> Job:
+@router.get("/{job_spec}")


what will a spec be in this case? we could consider a more descriptive variable name here

i.e. are we changing this from a UUID?

We are. It can be a uuid (returning a single job) or a job type name (inference or evaluate, returning a list of jobs). It follows the approach in the create_job service for consistency. I'd rather use a param (actually inheritance via anyOf plus a discriminator).

I'd prefer to have one route return one type of object for clarity and consistency. If we have a spec that returns multiple objects, we should return a dictionary of elements.

We have a couple of constraints: make it backwards compatible, and follow a similar interface to the existing calls.
Regarding the dictionary, the GET /jobs call already returns a list, so I'd see it natural to return a list as well.
Regarding the routes, I can use a more specific route like GET /jobs/per_type/{job_type} to avoid overloading the /jobs/{job_id} route. Wdyt?

We can actually use /jobs/{uuid} and /jobs/inference/ / /jobs/evaluate/ with trailing slashes :-/ It's a bit messy too but I kinda like it better.

I agreed with @aittalam on using /jobs/{job_type}/ to keep it consistent with the existing POST methods. Let me know if this would be ok for you.

veekaybee · 2025-01-08T17:04:38Z

lumigator/python/mzai/backend/backend/api/routes/jobs.py

+        ray_job = _get_ray_job(job_id)
+
+        # Combine both types of response.
+        x = ray_job.model_dump()  # JobSubmissionResponse


if we're looking to understand the type here, we can annotate it:

x: JobSubmissionResponse = ray_job.model_dump()

Hmmm, I'm not sure I wrote that, but type annotations are fine for me. IIRC this would need to be taken from ray (but we do have those dependencies and can import the types, right?).

I'm not sure I wrote that

I mean I just copied the existing approach of merging the Ray info together with our own.

lumigator/python/mzai/backend/backend/api/routes/jobs.py

lumigator/python/mzai/backend/backend/services/jobs.py

lumigator/python/mzai/backend/backend/api/routes/jobs.py

veekaybee · 2025-01-09T15:00:03Z

lumigator/python/mzai/backend/backend/api/routes/jobs.py

+
+        # Merge Ray jobs into the repositories jobs
+        for job in jobs.items:
+            found_job = next(


Not sure I understand this piece of logic: why are we looking to combine Ray job and lumigator IDs here?

This was done in a separate PR already, I'm just following it. The Ray info is bundled with the Lumigator job info so the UI has all the info in one place. Please check #514

veekaybee · 2025-01-09T16:02:01Z

...gator/python/mzai/backend/backend/alembic/versions/ef5ee5662ce3_add_job_type_to_job_table.py

+    op.add_column("jobs", sa.Column("job_type", sa.String(), nullable=True))
+
+
+def downgrade() -> None:


are we adding and removing the same column here?

If I understand correctly (which may not be doing!) adding the column is part of the upgrade procedure into this db revision from the previous, and removing the column is part of the downgrade procedure from this db revision into the previous. But maybe I'm not getting it right :-/

I confirm! This is automatic code generated by alembic to upgrade / downgrade the database to a new / older version.
As Dimitri's PR has been merged, I think you don't need to have this anymore as you are not actively modifying the DB (IIRC)

I stand corrected, the previous PR was not dealing with thejob_type field. You'll need to run alembic again then and generate a more recent migration if you have not done it already

veekaybee · 2025-01-15T15:49:53Z

...gator/python/mzai/backend/backend/alembic/versions/ef5ee5662ce3_add_job_type_to_job_table.py

+revision: str = "ef5ee5662ce3"  # pragma: allowlist secret
+down_revision: str | None = "e9679cbc3c36"  # pragma: allowlist secret
+branch_labels: str | Sequence[str] | None = None
+depends_on: str | Sequence[str] | None = None


Let's make sure when we merge this it doesn't conflict with other alembic changes

Hmmmmm, true :-/ I'll regenerate the alembic part just before merging just in case.

veekaybee · 2025-01-15T15:57:10Z

lumigator/python/mzai/backend/backend/api/routes/jobs.py

@@ -111,15 +111,15 @@ def list_jobs(
    # Merge Ray jobs into the repositories jobs
    for job in jobs.items:
        found_job = next(


I'd suggest splitting this code into two lines and commenting for legibility, and in splitting it up, looks like this can go into a dict?

return_val: type = (job for job in filter(lambda x: x.submission_id == str(job.id), ray_jobs)), None found_job: [type here] = next(return_val)

suggested refactor:

ray_jobs = {} for job in jobs.items(): submission_id = job[1].submission_id if submission_id: ray_jobs[submission_id] = job[1]

Handling a set/dict to keep track of the ids definitely helps finding out ray/job relations.
Ok, refactoring 👍

Did something similar:

# Get all jobs Ray knows about on a dict ray_jobs = {ray_job.submission_id: ray_job for ray_job in _get_all_ray_jobs()} results = list[Job]() # Merge Ray jobs into the repositories jobs job: JobResponse for job in jobs.items: job_id = str(job.id) if job_id in ray_jobs: # Combine both types of response.

lumigator/python/mzai/backend/backend/api/routes/jobs.py

veekaybee · 2025-01-15T15:59:16Z

lumigator/python/mzai/backend/backend/api/routes/jobs.py

+    """Attempts to retrieve merged job data from the Lumigator repository and Ray
+    for a valid UUID.
+
+    The result is a merged representation which forms an augmented view of a 'job'.


looks like the comment string is incorrectly formatted

In what sense? I don't get the issue :(

veekaybee · 2025-01-15T15:59:43Z

lumigator/python/mzai/backend/backend/api/routes/jobs.py

-    x = ray_job.model_dump()  # JobSubmissionResponse
-    y = job.model_dump()  # JobResponse
-    merged = {**x, **y}
+    ray_info = ray_job.dict()


would recommend creating or using an exiting PyDantic class for this

We do. It's just an older version that Ray uses, afaict :-/ I tried using model_dump, but if I got it right it's their Pydantic version not implementing it.

lumigator/lumigator/python/mzai/backend/backend/api/routes/jobs.py

Line 243 in 1b5151e

def _get_all_ray_jobs() -> list[RayJobDetails]:

lumigator/lumigator/python/mzai/backend/backend/api/routes/jobs.py

Line 20 in 1b5151e

from ray.job_submission import JobDetails as RayJobDetails

https://github.com/ray-project/ray/blob/916f534e571278b26733812b24a7b3dee08f24e4/python/ray/dashboard/modules/job/pydantic_models.py#L38

lumigator/python/mzai/backend/backend/api/routes/jobs.py

veekaybee · 2025-01-15T16:25:01Z

lumigator/python/mzai/backend/backend/api/routes/jobs.py

@@ -194,7 +240,7 @@ def get_job_result_download(
    return service.get_job_result_download(job_id)


-def _get_all_ray_jobs() -> list[JobSubmissionResponse]:
+def _get_all_ray_jobs() -> list[RayJobDetails]:


why are we changing this?

Because the info we get is modelled by the Ray pydantic models. Then it is merged with our own job info.

The merged information is not modelled as JointJobInfo(RayJobDetails, JobResponse) because this would require the lumigator schemas to import the ray package, which is too heavy for the end user as there is no separate api or model package for Ray.

lumigator/python/mzai/backend/backend/repositories/jobs.py

veekaybee · 2025-01-15T16:27:06Z

lumigator/python/mzai/backend/backend/services/jobs.py

@@ -24,6 +24,9 @@
 from backend.services.datasets import DatasetService
 from backend.settings import settings

+DEFAULT_SKIP = 0


let's injecting per method call to follow the rest of the repo

lumigator/lumigator/python/mzai/sdk/lumigator_sdk/experiments.py

Line 41 in dca29c3

self, skip: int = 0, limit: int = 100

Yes, but I'd prefer to have defaults as consts and not magic numbers, to keep consistency over the code.

github-actions bot added backend api Changes which impact API/presentation layer labels Jan 7, 2025

javiermtorres force-pushed the javiermtorres/issue-380-list-per-job-type branch from 458d480 to eba403f Compare January 7, 2025 17:45

javiermtorres changed the title ~~Add db interface, tests~~ List jobs per job type Jan 8, 2025

github-actions bot added the sdk label Jan 8, 2025

javiermtorres marked this pull request as ready for review January 8, 2025 16:42

javiermtorres requested review from ividal, aittalam and veekaybee January 8, 2025 16:42

veekaybee reviewed Jan 8, 2025

View reviewed changes

lumigator/python/mzai/backend/backend/api/routes/jobs.py Outdated Show resolved Hide resolved

veekaybee reviewed Jan 8, 2025

View reviewed changes

lumigator/python/mzai/backend/backend/api/routes/jobs.py Outdated Show resolved Hide resolved

aittalam reviewed Jan 9, 2025

View reviewed changes

lumigator/python/mzai/backend/backend/api/routes/jobs.py Outdated Show resolved Hide resolved

aittalam reviewed Jan 9, 2025

View reviewed changes

lumigator/python/mzai/backend/backend/services/jobs.py Show resolved Hide resolved

veekaybee reviewed Jan 9, 2025

View reviewed changes

lumigator/python/mzai/backend/backend/api/routes/jobs.py Outdated Show resolved Hide resolved

veekaybee reviewed Jan 9, 2025

View reviewed changes

lumigator/python/mzai/backend/backend/api/routes/jobs.py Outdated Show resolved Hide resolved

veekaybee reviewed Jan 9, 2025

View reviewed changes

javiermtorres added 5 commits January 10, 2025 11:35

Add db interface, tests

ad44ed0

Add services, routes

8440d53

Add sdk

ddf825c

Add docs

3cf3d1b

Review rework

0197eac

javiermtorres force-pushed the javiermtorres/issue-380-list-per-job-type branch from 238cbb1 to 0197eac Compare January 10, 2025 14:02

javiermtorres added 2 commits January 13, 2025 08:49

Merge branch 'main' into javiermtorres/issue-380-list-per-job-type

d8edaca

Review rework

0dad580

github-actions bot added the schemas Changes to schemas (which may be public facing) label Jan 13, 2025

Merge branch 'main' into javiermtorres/issue-380-list-per-job-type

1b5151e

veekaybee reviewed Jan 15, 2025

View reviewed changes

lumigator/python/mzai/backend/backend/api/routes/jobs.py Show resolved Hide resolved

veekaybee reviewed Jan 15, 2025

View reviewed changes

lumigator/python/mzai/backend/backend/api/routes/jobs.py Show resolved Hide resolved

veekaybee reviewed Jan 15, 2025

View reviewed changes

lumigator/python/mzai/backend/backend/repositories/jobs.py Show resolved Hide resolved

veekaybee reviewed Jan 15, 2025

View reviewed changes

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

List jobs per job type #576

List jobs per job type #576

javiermtorres commented Jan 7, 2025 •

edited

Loading

veekaybee Jan 8, 2025

veekaybee Jan 8, 2025

javiermtorres Jan 8, 2025

veekaybee Jan 9, 2025

javiermtorres Jan 9, 2025

javiermtorres Jan 9, 2025

javiermtorres Jan 13, 2025

veekaybee Jan 8, 2025

javiermtorres Jan 8, 2025

javiermtorres Jan 9, 2025

veekaybee Jan 9, 2025

javiermtorres Jan 9, 2025

veekaybee Jan 9, 2025

javiermtorres Jan 9, 2025

aittalam Jan 15, 2025

aittalam Jan 16, 2025

veekaybee Jan 15, 2025

javiermtorres Jan 15, 2025

veekaybee Jan 15, 2025 •

edited

Loading

javiermtorres Jan 15, 2025

javiermtorres Jan 16, 2025 •

edited

Loading

veekaybee Jan 15, 2025

javiermtorres Jan 16, 2025

veekaybee Jan 15, 2025

javiermtorres Jan 15, 2025

veekaybee Jan 15, 2025

javiermtorres Jan 15, 2025

javiermtorres Jan 16, 2025

veekaybee Jan 15, 2025

javiermtorres Jan 15, 2025

		op.add_column("jobs", sa.Column("job_type", sa.String(), nullable=True))


		def downgrade() -> None:

List jobs per job type #576

Are you sure you want to change the base?

List jobs per job type #576

Conversation

javiermtorres commented Jan 7, 2025 • edited Loading

What's changing

How to test it

Additional notes for reviewers

I already...

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

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

Choose a reason for hiding this comment

Choose a reason for hiding this comment

veekaybee Jan 15, 2025 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

javiermtorres Jan 16, 2025 • edited Loading

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

Choose a reason for hiding this comment

Choose a reason for hiding this comment

javiermtorres commented Jan 7, 2025 •

edited

Loading

veekaybee Jan 15, 2025 •

edited

Loading

javiermtorres Jan 16, 2025 •

edited

Loading