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.

Sign up for GitHub

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

Jump to bottom

Connect multiple jobs under one experiment #592

Open
wants to merge 3 commits into
base: main
Choose a base branch
from
Open

Connect multiple jobs under one experiment #592

wants to merge 3 commits into from
+177 −14

Conversation

dpoulopoulos
Copy link
Contributor

@dpoulopoulos dpoulopoulos commented Jan 10, 2025 •
edited
Loading

What's changing

This PR adds a new experiment table in the SQLite database, a new column called experiment_id in the jobs table, and links multiple jobs under one experiment, using a foreign key constraint to model the many-to-one relationship between jobs and experiment.

Refs #570

How to test it

Steps to test the changes:

  1. Deploy Lumigator with make local-up.
  2. Upload a new dataset.
  3. Create a new experiment, using the /experiments_new endpoint. Note the experiment ID.
  4. Get all the jobs, and verify the the experiment_id attribute is set with the ID you got in the previous step.

I already...

    • Tested the changes in a working environment to ensure they work as expected
    • Added some tests for any new functionality
  • [NA] 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

@dpoulopoulos dpoulopoulos linked an issue Jan 10, 2025 that may be closed by this pull request
Add experiment table + experiment_id to jobs #570
Open
@dpoulopoulos dpoulopoulos self-assigned this Jan 10, 2025
@github-actions github-actions bot added backend api Changes which impact API/presentation layer schemas Changes to schemas (which may be public facing) labels Jan 10, 2025
@dpoulopoulos dpoulopoulos removed the schemas Changes to schemas (which may be public facing) label Jan 10, 2025
@dpoulopoulos dpoulopoulos force-pushed the 570-connect-jobs-experiments branch from bcb3538 to bf865a0 Compare January 10, 2025 09:44
@github-actions github-actions bot added the schemas Changes to schemas (which may be public facing) label Jan 10, 2025
@dpoulopoulos
db: Introduce experiments and link jobs (#570)
923720e 
Introduce the `experiments` table and link the `jobs` table to the
`experiments` table using a new foreign key constraint. Connect multiple
jobs under one experiment.

Refs #570

Signed-off-by: Dimitris Poulopoulos <dimitris@mozilla.ai>
@dpoulopoulos
Backend: Link multiple jobs to a single experiment (#570)
aff629f 
Create a new experiment instance in the database and link each job the
new experiment spawns to it.

Refs #570

Signed-off-by: Dimitris Poulopoulos <dimitris@mozilla.ai>
@dpoulopoulos dpoulopoulos force-pushed the 570-connect-jobs-experiments branch from bf865a0 to 695dfbe Compare January 10, 2025 09:46
@dpoulopoulos dpoulopoulos marked this pull request as ready for review January 10, 2025 09:46
@dpoulopoulos dpoulopoulos requested a review from aittalam January 10, 2025 09:47
@dpoulopoulos dpoulopoulos changed the title 570 connect jobs experiments Connect multiple jobs under one experiment Jan 10, 2025
javiermtorres
javiermtorres reviewed Jan 10, 2025
View reviewed changes
Copy link
Contributor

@javiermtorres javiermtorres left a comment

Choose a reason for hiding this comment

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

I'd miss a unit test to verify the whole experiment-job creation and link chain. We can use ray to provide mock requests for job creation. Otherwise, we can move that kind of test to integration.

@dpoulopoulos
tests: Add unit tests for the experiment repo (#570)
4b65a85 
Refs #570

Signed-off-by: Dimitris Poulopoulos <dimitris@mozilla.ai>
@dpoulopoulos dpoulopoulos force-pushed the 570-connect-jobs-experiments branch from 695dfbe to 4b65a85 Compare January 10, 2025 10:21
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@javiermtorres javiermtorres javiermtorres left review comments

@aittalam aittalam Awaiting requested review from aittalam

@veekaybee veekaybee Awaiting requested review from veekaybee

At least 1 approving review is required to merge this pull request.

Assignees

@dpoulopoulos dpoulopoulos

Labels
api Changes which impact API/presentation layer backend schemas Changes to schemas (which may be public facing)
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

Add experiment table + experiment_id to jobs
2 participants
@dpoulopoulos @javiermtorres