backend: rework queue::push after v2

[uael]

Important

Refactor job queue handling by introducing RawJob struct, moving benchmark code to windmill-worker, and updating job processing logic.

• Behavior:
  • Introduces RawJob struct in windmill-queue/src/jobs.rs to encapsulate job details and streamline job insertion.
  • Refactors job insertion logic in push() and push_many() functions in windmill-queue/src/jobs.rs.
  • Updates job processing logic in add_batch_jobs() in windmill-api/src/jobs.rs to use RawJob.
• Benchmarking:
  • Moves benchmark-related code from windmill-common/src/bench.rs to windmill-worker/src/bench.rs.
  • Updates benchmark initialization in run_worker() in windmill-worker/src/worker.rs.
• Misc:
  • Adds new SQL queries for job handling in .sqlx files.
  • Renames concurrent_time_window_s to concurrency_time_window_s in windmill-api/src/jobs.rs and windmill-queue/src/jobs.rs.
  • Removes bench.rs from windmill-common and updates references to it in windmill-worker.

^{This description was created by for 084dc6e. It will automatically update as commits are pushed.}

[cloudflare-workers-and-pages]

Deploying windmill with    Cloudflare Pages

Latest commit:	bd57e7f
Status:	✅  Deploy successful!
Preview URL:	https://97e27e67.windmill.pages.dev
Branch Preview URL:	https://uael-v2-rework-push.windmill.pages.dev

View logs

[alwaysmeticulous]

Meticulous was unable to execute a test run for this PR because the most recent commit is associated with multiple PRs. To execute a test run, please try pushing up a new commit that is only associated with this PR.

_{Last updated for commit 16bf861. This comment will update as new commits are pushed.}

[ellipsis-dev]

👍 Looks good to me! Reviewed everything up to 084dc6e in 2 minutes and 11 seconds

More details

• Looked at 1302 lines of code in 16 files
• Skipped 0 files when reviewing.
• Skipped posting 17 drafted comments based on config settings.

1. backend/windmill-worker/src/worker_flow.rs:87

• Draft comment:
  The function update_flow_status_after_job_completion uses a manual tail-recursive loop (with a loop and repeated calls to potentially_crash_for_testing). Consider refactoring into smaller helper functions or using proper async recursion (with caution regarding stack usage) to improve readability and maintainability.
• Reason this comment was not posted:
  Comment was not on a location in the diff, so it can't be submitted as a review comment.

2. backend/windmill-worker/src/worker_flow.rs:219

• Draft comment:
  Multiple calls to unwrap() and expect() (e.g. in payload_from_modules and similar functions) may panic if the expected data isn’t present. Consider handling these Result/Option cases more gracefully in production code.
• Reason this comment was not posted:
  Comment was not on a location in the diff, so it can't be submitted as a review comment.

3. backend/windmill-worker/src/worker_flow.rs:1290

• Draft comment:
  The function compute_bool_from_expr converts the evaluation result to a string and then compares with "true" or "false". This approach is brittle – consider returning a proper boolean value from eval_timeout or parsing the output using a JSON boolean.
• Reason this comment was not posted:
  Comment was not on a location in the diff, so it can't be submitted as a review comment.

4. backend/windmill-worker/src/worker_flow.rs:2655

• Draft comment:
  The insertion of iterative arguments via insert_iter_arg recursively changes the key name if conflicts are found. Although clever, it could lead to unexpected key names if deeply nested conflicts occur. Please document and test this behavior.
• Reason this comment was not posted:
  Comment was not on a location in the diff, so it can't be submitted as a review comment.

5. backend/windmill-worker/src/worker_flow.rs:3200

• Draft comment:
  The functions raw_script_to_payload and script_to_payload assemble payloads using multiple unwrap_or and manual merging of options. Consider extracting common routines and clarifying the override logic so future maintainers can more easily follow the intended precedence.
• Reason this comment was not posted:
  Comment was not on a location in the diff, so it can't be submitted as a review comment.

6. backend/windmill-worker/src/worker_flow.rs:4040

• Draft comment:
  When retrieving previous job results, the code uses query_scalar! and then directly unwraps values. Consider validating the result more carefully and returning a meaningful error if data is missing.
• Reason this comment was not posted:
  Comment was not on a location in the diff, so it can't be submitted as a review comment.

7. backend/windmill-worker/src/worker_flow.rs:1360

• Draft comment:
  The eval_timeout call in compute_bool_from_expr passes many optional context parameters. It might be cleaner to encapsulate evaluation and context building in a helper function to reduce duplication and potential errors.
• Reason this comment was not posted:
  Comment was not on a location in the diff, so it can't be submitted as a review comment.

8. backend/windmill-worker/src/worker_flow.rs:3480

• Draft comment:
  The branch handling code (FlowModuleValue::BranchOne and BranchAll) is complex. Consider splitting it into smaller dedicated helper functions with clear responsibilities.
• Reason this comment was not posted:
  Comment was not on a location in the diff, so it can't be submitted as a review comment.

9. backend/windmill-worker/src/worker_flow.rs:3572

• Draft comment:
  There is substantial logic for handling for-loop iterations. The next_loop_iteration function is very long and nested. Consider refactoring parts of the logic into helper functions (e.g., extracting context preparation, iterated array re-computation) for improved clarity.
• Reason this comment was not posted:
  Comment was not on a location in the diff, so it can't be submitted as a review comment.

10. backend/windmill-worker/src/worker_flow.rs:70

• Draft comment:
  The async function 'update_flow_status_after_job_completion' contains a loop that manually recurses until a terminal state is reached. Consider adding a safeguard (e.g. a maximum iteration count or timeout) to prevent an infinite loop if the state never converges.
• Reason this comment was not posted:
  Comment was not on a location in the diff, so it can't be submitted as a review comment.

11. backend/windmill-worker/src/worker_flow.rs:218

• Draft comment:
  In 'update_flow_status_after_job_completion_internal', several SQL queries and JSON payload extractions use manual parsing with serde_json::from_str. Consider using sqlx’s built‐in JSON deserialization support to improve clarity and error handling.
• Reason this comment was not posted:
  Comment was not on a location in the diff, so it can't be submitted as a review comment.

12. backend/windmill-worker/src/worker_flow.rs:430

• Draft comment:
  This function, 'update_flow_status_after_job_completion_internal', is very lengthy and complex. Consider refactoring parts of this function into smaller helper functions to improve readability and maintainability.
• Reason this comment was not posted:
  Comment was not on a location in the diff, so it can't be submitted as a review comment.

13. backend/windmill-worker/src/worker_flow.rs:1330

• Draft comment:
  In 'compute_bool_from_expr', the evaluated expression is wrapped into a Boolean constructor and compared with string literals 'true' or 'false'. This approach may be fragile if the evaluator returns non‐string boolean values. Consider a more robust check or explicit conversion.
• Reason this comment was not posted:
  Comment was not on a location in the diff, so it can't be submitted as a review comment.

14. backend/windmill-worker/src/worker_flow.rs:398

• Draft comment:
  The 'needs_resume' function uses a terse check on 'required_events'. Verify that this logic correctly handles cases when the suspend configuration is mis‐configured (e.g. accidentally set to 0) and add comments to clarify the intent.
• Reason this comment was not posted:
  Comment was not on a location in the diff, so it can't be submitted as a review comment.

15. backend/windmill-worker/src/worker_flow.rs:3597

• Draft comment:
  The helper function 'is_simple_modules' has a long list of conditions determining the simplicity of a module. Consider adding inline comments or breaking up the logic to clearly document the criteria.
• Reason this comment was not posted:
  Comment was not on a location in the diff, so it can't be submitted as a review comment.

16. backend/windmill-worker/src/worker_flow.rs:1210

• Draft comment:
  There are many SQL queries using inline raw SQL strings with repetitive patterns. Consider extracting these queries into helper functions or constants to improve maintainability and reduce duplication.
• Reason this comment was not posted:
  Comment was not on a location in the diff, so it can't be submitted as a review comment.

17. backend/windmill-worker/src/worker_flow.rs:3750

• Draft comment:
  The functions 'next_loop_iteration', 'payload_from_simple_module', and similar payload builders are heavily nested and mix business logic with payload construction. It would improve testability and clarity to separate the payload construction from the control‐flow logic.
• Reason this comment was not posted:
  Comment was not on a location in the diff, so it can't be submitted as a review comment.

Workflow ID: wflow_gaop5XtfLRcovVgz

---

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