Skip to content
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 pagination and partial name search to List Jobs APIs #581

Merged
merged 16 commits into from
May 21, 2024

Conversation

krithika369
Copy link
Collaborator

@krithika369 krithika369 commented May 15, 2024

Description

This PR adds 2 new paginated APIs for listing jobs:

  • /projects/{project_id}/jobs-by-page
  • /models/{model_id}/versions/{version_id}/jobs-by-page

With this, the use of the existing .../jobs list APIs has been replaced by the new APIs in the SDK implementation and will also be replaced for the UI, in another PR. The non-paginated list jobs APIs have been marked deprecated and can be removed eventually.

Illustration

Screenshot 2024-05-16 at 7 11 21 AM

Modifications

  • swagger.yaml
    • Deprecate existing list jobs APIs.
    • Add new /jobs-by-page APIs. These APIs accept a new search query parameter compared to the existing list jobs APIs. This parameter will do partial matches of the job name as opposed to equality matching. This can be particularly useful for searches done from the UI. (The search parameter has been named so, taking inspiration from XP - example.)
  • api/api/prediction_job_api.go - Implement the paginated APIs
  • api/service/prediction_job_service.go
    • Add paginator to predictionJobService.
    • Add Page, PageSize and Search parameters to ListPredictionJobQuery.
    • Add isPaginated flag to ListPredictionJobs method. When set, the DB query will be executed with the appropriate offset and limit and the pagination data sent back.
  • api/storage/prediction_job_storage.go - Add Count method. The List method is updated to handle offset and limit. Both methods support partial searching of the name column.
  • api/go.mod - Update MLP API dependency to consume Add package pagination mlp#94 and Add JSON tags for Paging struct mlp#98
  • python/sdk/merlin/model.py - Update the list prediction job method to use the paginated backed API

Tests

SDK's list_prediction_job API (which lists the jobs for a given model version) now uses the paginated backend API, getting a maximum of 10 results at once. The performance of this has been tested locally with a dataset size of 275 (results in 28 jobs API calls). This does make the SDK method slower (0.33 seconds vs 0.85 seconds locally). If required, in the future, the page_size passed to the API call can be explicitly set to a larger value or the pagination options can be exposed to the user via the SDK.

Checklist

  • Added PR label
  • Added unit test, integration, and/or e2e tests
  • Tested locally
  • Updated documentation
  • Update Swagger spec if the PR introduce API changes
  • Regenerated Golang and Python client if the PR introduces API changes

Release Notes

Add new paginated APIs for listing prediction jobs.

@krithika369 krithika369 added the enhancement New feature or request label May 15, 2024
@krithika369 krithika369 marked this pull request as draft May 15, 2024 03:46
@krithika369 krithika369 changed the title Add pagination and partial name search capabilities to List Jobs APIs Add pagination and partial name search to List Jobs APIs May 15, 2024
Copy link

codecov bot commented May 15, 2024

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 60.39%. Comparing base (9cdcecb) to head (bfc08e1).
Report is 2 commits behind head on main.

Additional details and impacted files
@@            Coverage Diff             @@
##             main     #581      +/-   ##
==========================================
+ Coverage   60.31%   60.39%   +0.08%     
==========================================
  Files         274      274              
  Lines       21930    22017      +87     
==========================================
+ Hits        13226    13298      +72     
- Misses       7855     7863       +8     
- Partials      849      856       +7     
Flag Coverage Δ
api-test 58.33% <ø> (+0.09%) ⬆️
sdk-test-3.10 75.54% <ø> (+0.06%) ⬆️
sdk-test-3.8 75.46% <ø> (+0.06%) ⬆️
sdk-test-3.9 75.46% <ø> (+0.06%) ⬆️

Flags with carried forward coverage won't be shown. Click here to find out more.

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

@krithika369 krithika369 marked this pull request as ready for review May 16, 2024 03:15
Copy link
Contributor

@ariefrahmansyah ariefrahmansyah left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

lgtm. thanks @krithika369 !

@ariefrahmansyah ariefrahmansyah merged commit afa408f into main May 21, 2024
36 checks passed
@ariefrahmansyah ariefrahmansyah deleted the list_jobs_pagination branch May 21, 2024 05:29
ariefrahmansyah added a commit that referenced this pull request May 21, 2024
<!--  Thanks for sending a pull request!  Here are some tips for you:

1. Run unit tests and ensure that they are passing
2. If your change introduces any API changes, make sure to update the
e2e tests
3. Make sure documentation is updated for your PR!

-->
# Description
<!-- Briefly describe the motivation for the change. Please include
illustrations where appropriate. -->
This PR is based on the corresponding API PR
#581 (once that is merged to
`main`, the base branch for this PR will be updated).

The per model version jobs list view now uses the new `/jobs-by-page`
paginated API. With this, the UI now enables pagination which is carried
out on the backend. Also, partial searching of the job name is possible
from the UI and this search is also carried out on the backend, using
the same API endpoint.

## Illustration


https://github.com/caraml-dev/merlin/assets/23465343/5a2ce290-3d45-4a10-b223-34d2e4e44816

# Modifications
<!-- Summarize the key code changes. -->
* `ui/src/config.js` - Introduce pagination page size config
* `ui/src/job/JobListTable.js` - Replace Eui in-memory table with a
basic table and implement the search bar and pagination
* `ui/src/job/Jobs.js` - Update backend API call

# Tests
<!-- Besides the existing / updated automated tests, what specific
scenarios should be tested? Consider the backward compatibility of the
changes, whether corner cases are covered, etc. Please describe the
tests and check the ones that have been completed. Eg:
- [x] Deploying new and existing standard models
- [ ] Deploying PyFunc models
-->
- [x] Changes to search text resets pagination (back to page 1)

# Checklist
- [x] Added PR label
- [ ] Added unit test, integration, and/or e2e tests
- [x] Tested locally
- [ ] Updated documentation
- [ ] Update Swagger spec if the PR introduce API changes
- [ ] Regenerated Golang and Python client if the PR introduces API
changes

# Release Notes
<!--
Does this PR introduce a user-facing change?
If no, just write "NONE" in the release-note block below.
If yes, a release note is required. Enter your extended release note in
the block below.
If the PR requires additional action from users switching to the new
release, include the string "action required".

For more information about release notes, see kubernetes' guide here:
http://git.k8s.io/community/contributors/guide/release-notes.md
-->

```release-note
Add pagination to the List Jobs UI
```

---------

Co-authored-by: Arief Rahmansyah <ariefrahmansyah@hotmail.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants