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

feat(backend): move some static fields out of queue #4631

Open
wants to merge 1 commit into
base: main
Choose a base branch
from

Conversation

uael
Copy link
Contributor

@uael uael commented Nov 4, 2024

This commit move raw_* fields from the queue table to a new
job_definition table. For backward compatibility, they are left
unchanged in the queue table, hence all parameters fetch use coalesce
or a fallback to the queue table. As such, all previously pushed jobs
(before migration) will work as intended.

Important

This PR introduces a job_params table to store raw_code and raw_flow separately, optimizing database operations and updating related code to use this new structure.

  • Database Changes:
    • Introduce job_params table in 20241004112544_create_params_args_result_tables.up.sql to store raw_code and raw_flow.
    • Update 20241004112544_create_params_args_result_tables.down.sql to drop job_params table.
  • Code Changes:
    • Add QueuedJobEx struct in jobs.rs to extend QueuedJob with raw_code and raw_flow.
    • Replace get_queued_job with get_queued_job_ex in jobs.rs and other files to fetch extended job data.
    • Modify handle_dependency_job and handle_flow_dependency_job in worker_lockfiles.rs to use job_params for raw_code and raw_flow.
  • Query Updates:
    • Update SQL queries in jobs.rs and worker_flow.rs to join job_params with queue for fetching raw_code and raw_flow.
    • Adjust generate_get_job_query in jobs.rs to include job_params fields.
  • Miscellaneous:
    • Remove raw_code and raw_flow from QueuedJob struct in jobs.rs.
    • Update handle_code_execution_job in worker.rs to use job_params for raw_code.

This description was created by Ellipsis for 12f7d6a. It will automatically update as commits are pushed.

Copy link
Contributor

github-actions bot commented Nov 4, 2024

CLA Assistant Lite bot All contributors have signed the CLA ✍️ ✅

backend/windmill-api/src/jobs.rs Outdated Show resolved Hide resolved
backend/windmill-api/src/jobs.rs Outdated Show resolved Hide resolved
@uael uael force-pushed the uael/split_queue branch 6 times, most recently from 7268cb0 to 730dbc2 Compare November 5, 2024 08:50
@uael uael force-pushed the uael/split_queue branch 4 times, most recently from db23339 to 12f7d6a Compare November 5, 2024 09:16
@uael uael marked this pull request as ready for review November 5, 2024 09:17
@uael
Copy link
Contributor Author

uael commented Nov 5, 2024

I have read the CLA Document and I hereby sign the CLA

github-actions bot added a commit that referenced this pull request Nov 5, 2024
Copy link
Contributor

@ellipsis-dev ellipsis-dev bot left a comment

Choose a reason for hiding this comment

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

👍 Looks good to me! Reviewed everything up to 12f7d6a in 2 minutes and 15 seconds

More details
  • Looked at 901 lines of code in 10 files
  • Skipped 0 files when reviewing.
  • Skipped posting 6 drafted comments based on config settings.
1. backend/windmill-worker/src/worker_lockfiles.rs:204
  • Draft comment:
    The is_some_and method is not available in stable Rust. Consider using map and unwrap_or for better compatibility and readability.
  • Reason this comment was not posted:
    Comment was on unchanged code.
2. backend/windmill-worker/src/worker_flow.rs:308
  • Draft comment:
    The is_some_and method is not available in stable Rust. Consider using map and unwrap_or for better compatibility and readability.
  • Reason this comment was not posted:
    Marked as duplicate.
3. backend/windmill-worker/src/worker_flow.rs:937
  • Draft comment:
    The is_some_and method is not available in stable Rust. Consider using map and unwrap_or for better compatibility and readability.
  • Reason this comment was not posted:
    Marked as duplicate.
4. backend/windmill-worker/src/worker_flow.rs:981
  • Draft comment:
    The is_some_and method is not available in stable Rust. Consider using map and unwrap_or for better compatibility and readability.
  • Reason this comment was not posted:
    Marked as duplicate.
5. backend/windmill-worker/src/worker_flow.rs:1049
  • Draft comment:
    The is_some_and method is not available in stable Rust. Consider using map and unwrap_or for better compatibility and readability.
  • Reason this comment was not posted:
    Marked as duplicate.
6. backend/windmill-worker/src/worker_flow.rs:1114
  • Draft comment:
    The is_some_and method is not available in stable Rust. Consider using map and unwrap_or for better compatibility and readability.
  • Reason this comment was not posted:
    Marked as duplicate.

Workflow ID: wflow_9L6CEztEYwwCZe5A


You can customize Ellipsis with 👍 / 👎 feedback, review rules, user-specific overrides, quiet mode, and more.

@uael uael changed the title wip: split out raw_code and raw_flow from queue db: move job parameters out of the queue table Nov 5, 2024
@uael uael force-pushed the uael/split_queue branch 2 times, most recently from 4decc53 to 6ef7f61 Compare November 5, 2024 09:55
@uael uael changed the title db: move job parameters out of the queue table feat(backend): move job parameters out of the queue table Nov 5, 2024
@uael uael force-pushed the uael/split_queue branch 2 times, most recently from e14c4dd to 0d90a0e Compare November 6, 2024 16:02
@uael uael changed the title feat(backend): move job parameters out of the queue table feat(backend): move some static fields out of queue Nov 6, 2024
This commit move `raw_*` fields from the `queue` table to a new
`job_definition` table. For backward compatibility, they are left
unchanged in the queue table, hence all parameters fetch use `coalesce`
or a fallback to the `queue` table. As such, all previously pushed jobs
(before migration) will work as intended.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

1 participant