Skip to content

feat: Move Press Jobs To Code#6159

Merged
tanmoysrt merged 28 commits intofrappe:developfrom
tanmoysrt:move_press_job_to_code
Apr 25, 2026
Merged

feat: Move Press Jobs To Code#6159
tanmoysrt merged 28 commits intofrappe:developfrom
tanmoysrt:move_press_job_to_code

Conversation

@tanmoysrt
Copy link
Copy Markdown
Member

@tanmoysrt tanmoysrt commented Apr 16, 2026

Resolves #5946

  • Add Support for callbacks in Workflow Engine
    • Add defer_current_task method to retry current task (Fix the occurrences in Release Pipeline also)
  • Refactor Press Job with Workflow Engine integration
  • Move all press jobs to code
  • Fix the tests
  • Perform Manual Tests
    • Check for durability
    • Check Permission Issue
    • Make all the Press Jobs idempotent in nature (For example, if machine provisioning done, dont reattempt based on the vm state)
  • Add support for retry / force fail
  • Add support for for JSON based serialization / deserialization

@codecov
Copy link
Copy Markdown

codecov Bot commented Apr 16, 2026

Codecov Report

❌ Patch coverage is 70.26210% with 590 lines in your changes missing coverage. Please review.
✅ Project coverage is 56.24%. Comparing base (3a0f2e1) to head (305542a).
⚠️ Report is 111 commits behind head on develop.

Files with missing lines Patch % Lines
...ress/press/doctype/press_job/jobs/create_server.py 44.69% 125 Missing ⚠️
...ress/press/doctype/press_job/jobs/snapshot_disk.py 32.43% 50 Missing ⚠️
...press/doctype/press_job/jobs/increase_disk_size.py 30.64% 43 Missing ⚠️
...s/doctype/press_job/jobs/create_server_snapshot.py 34.48% 38 Missing ⚠️
...ess/press/doctype/press_job/jobs/archive_server.py 29.16% 34 Missing ⚠️
...s/doctype/press_job/jobs/setup_on_prem_failover.py 49.23% 33 Missing ⚠️
...ss/doctype/press_job/jobs/stop_and_start_server.py 36.36% 28 Missing ⚠️
press/press/doctype/server/server.py 22.85% 27 Missing ⚠️
...ow_engine/doctype/press_workflow/press_workflow.py 59.67% 25 Missing ⚠️
...ress/press/doctype/press_job/jobs/resize_server.py 74.39% 21 Missing ⚠️
... and 27 more
Additional details and impacted files
@@             Coverage Diff             @@
##           develop    #6159      +/-   ##
===========================================
+ Coverage    55.93%   56.24%   +0.31%     
===========================================
  Files          910      934      +24     
  Lines        75826    77582    +1756     
  Branches       525      525              
===========================================
+ Hits         42412    43638    +1226     
- Misses       33386    33916     +530     
  Partials        28       28              
Flag Coverage Δ
dashboard 90.75% <ø> (+<0.01%) ⬆️

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.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
  • 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

@tanmoysrt tanmoysrt force-pushed the move_press_job_to_code branch 7 times, most recently from 39b420b to 91d36d8 Compare April 16, 2026 21:04
@tanmoysrt tanmoysrt force-pushed the move_press_job_to_code branch 2 times, most recently from 27f1910 to cfca2e2 Compare April 16, 2026 21:18
@tanmoysrt tanmoysrt force-pushed the move_press_job_to_code branch from cfca2e2 to 2ac199b Compare April 16, 2026 21:19
@tanmoysrt tanmoysrt force-pushed the move_press_job_to_code branch 2 times, most recently from 06fb74d to 5da1414 Compare April 18, 2026 16:18
@tanmoysrt tanmoysrt force-pushed the move_press_job_to_code branch from 5da1414 to 01f375d Compare April 18, 2026 16:19
@tanmoysrt tanmoysrt force-pushed the move_press_job_to_code branch from 8961980 to 3fc42f0 Compare April 24, 2026 06:15
@tanmoysrt tanmoysrt marked this pull request as ready for review April 24, 2026 08:22
@greptile-apps
Copy link
Copy Markdown
Contributor

greptile-apps Bot commented Apr 24, 2026

Greptile Summary

This PR migrates all Press Jobs from database-stored job type definitions to code-based workflow classes, leveraging the existing WorkflowBuilder / PressWorkflow engine. It also extends the workflow engine with callback support (retry with exponential back-off), force-fail, and a serialize_and_store_value / deserialize_value pair to replace PressWorkflowObject links with plain JSON for primitive values.

The architecture is solid, but the migration from PressWorkflowObject to JSON storage is only half-applied, leaving two P0 paths that will prevent all jobs from executing:

  • press_workflow.py:run() still calls PressWorkflowObject.get_object(self.args) even though self.args now holds a JSON string — every workflow run fails immediately with a Fatal status.
  • press_workflow_task.py calls deserialize_value(self.args) with one argument; the function requires two (value_type, serialized_value), so every task with args raises TypeError.

Confidence Score: 2/5

Not safe to merge — two P0 deserialization bugs will prevent all press jobs from running in production.

Two independent P0 bugs exist on the hot path: (1) press_workflow.py:run() uses the old PressWorkflowObject.get_object() on args fields that now store JSON strings, immediately marking every workflow Fatal; (2) press_workflow_task.py calls deserialize_value with one argument instead of two, throwing TypeError on every task with args. Both together mean zero press jobs can complete under the new system. Additionally there is a P1 in press_job.js (wrong status string hides the Retry button) and a P1 AttributeError in increase_disk_size.py. The overall design and all 21 new job files are well-structured.

press/workflow_engine/doctype/press_workflow/press_workflow.py (P0 in run()), press/workflow_engine/doctype/press_workflow_task/press_workflow_task.py (P0 in run()), press/workflow_engine/doctype/press_workflow_task/press_workflow_task.json (schema mismatch), press/press/doctype/press_job/press_job.js (P1 wrong status string)

Important Files Changed

Filename Overview
press/workflow_engine/doctype/press_workflow/press_workflow.py Core workflow runner — adds callback execution, force-fail, and retry logic; contains P0 bug: run() still calls PressWorkflowObject.get_object(self.args) on args that are now JSON strings, marking every workflow Fatal before any task runs
press/workflow_engine/doctype/press_workflow_task/press_workflow_task.py Task executor — migrated from PressWorkflowObject to new deserialize_value, but calls it with only 1 arg instead of 2, causing TypeError for every task that has args or kwargs set
press/workflow_engine/doctype/press_workflow_task/press_workflow_task.json Schema still declares args/kwargs as Link fields pointing to PressWorkflowObject, but code now stores raw JSON strings there; kwargs_type is incorrectly Data instead of Select
press/press/doctype/press_job/press_job.js Simplified to show only a Retry button, but the condition status === 'Failed' never matches — the doctype uses 'Failure' — so the button is permanently hidden
press/press/doctype/press_job/jobs/increase_disk_size.py IncreaseDiskSize job port; contains P1 AttributeError bug: self.arguments_dict.labels.get("mountpoint") crashes when labels key is absent
press/workflow_engine/doctype/press_workflow/decorators.py Updated to store args/kwargs as JSON strings via serialize_and_store_value; the storage side is correct, but the corresponding load side in press_workflow.py was not updated
press/workflow_engine/utils.py New serialize/deserialize helpers for primitive and container types, with PressWorkflowObject fallback for non-serializable values; implementation is correct
press/press/doctype/press_job/jobs/create_server.py Largest job — port of CreateServer to code-based workflow; idempotent guard logic looks sound; callbacks update ServerSnapshotRecovery and logical replication records correctly
press/workflow_engine/doctype/press_workflow_kv/press_workflow_kv.py KV store migrated from PressWorkflowObject links to typed JSON values; get/set/delete logic is consistent with new serialize/deserialize helpers
press/hooks.py Removes legacy fail_stuck_press_jobs and process_failed_callbacks scheduled tasks; adds retry_workflow_callbacks at the 5-minute cadence

Sequence Diagram

sequenceDiagram
    participant PJ as PressJob
    participant WB as WorkflowBuilder
    participant BF as BoundFlow (@flow)
    participant PW as PressWorkflow
    participant PWT as PressWorkflowTask

    PJ->>WB: after_insert → start_workflow()
    WB->>BF: execute.run_as_workflow()
    BF->>BF: serialize_and_store_value(args) → ("tuple","[]")
    BF->>PW: insert(args="[]", args_type="tuple", ...)
    PW-->>PW: after_insert → enqueue_workflow()
    PW->>PW: run()
    Note over PW: ⚠️ P0: PressWorkflowObject.get_object("[]") → Fatal
    PW->>PWT: enqueue task → PressWorkflowTask.run()
    Note over PWT: ⚠️ P0: deserialize_value(self.args) missing 2nd arg → TypeError
    PWT-->>PWT: execute @task method on reference_doc
    PWT->>PW: _resume_workflow()
    PW->>PW: on_workflow_success/failure
    PW->>PJ: execute_callback → on_workflow_success/failure
    PJ->>PJ: status = "Success"/"Failure"
Loading

Comments Outside Diff (3)

  1. press/workflow_engine/doctype/press_workflow/press_workflow.py, line 100-101 (link)

    P0 Stale deserialization breaks all workflow runs

    self.args is now stored as a JSON string ("[]" for an empty tuple) in a Data field, but PressWorkflowObject.get_object("[]") tries to look up a Press Workflow Object document named "[]" — which will not exist. The surrounding except Exception block will catch this and immediately mark every workflow as Fatal before any job step can execute.

    The run() method needs to use the new deserialize_value(self.args_type, self.args) helper that was introduced for exactly this purpose (already imported as serialize_and_store_value from the same module; deserialize_value needs to be added to that import).

    # Fix: replace the stale PressWorkflowObject calls with the new helpers
    args = deserialize_value(self.args_type, self.args) if self.args else ()
    kwargs = deserialize_value(self.kwargs_type, self.kwargs) if self.kwargs else {}
  2. press/workflow_engine/doctype/press_workflow_task/press_workflow_task.json, line 148-158 (link)

    P1 args/kwargs fields are still Link type but now store JSON strings

    workflow_builder.py now sets task_doc.args = args_value where args_value is a raw JSON string (e.g. "[]" for an empty tuple), but the JSON schema still declares args and kwargs as Link fields pointing to Press Workflow Object. Frappe will attempt to resolve these as foreign-key links to documents that don't exist under those names, which can cause validation errors on insert and will certainly produce broken UI references.

    Both fields should be changed to Data (or Small Text) to match how the Press Workflow doctype updated its own args/kwargs fields in this PR.

  3. press/workflow_engine/doctype/press_workflow_task/press_workflow_task.json, line 213-222 (link)

    P2 kwargs_type declared as Data instead of Select

    args_type and output_type in the same file use fieldtype: "Select", but kwargs_type uses fieldtype: "Data". The "options" key is silently ignored on a Data field, so the value is unconstrained and the UI won't render a dropdown. It should be "Select" to match the other type fields.

Reviews (1): Last reviewed commit: "feat(workflow-engine): Store args and kw..." | Re-trigger Greptile

Comment thread press/workflow_engine/doctype/press_workflow_task/press_workflow_task.py Outdated
Comment thread press/press/doctype/press_job/press_job.js Outdated
Comment thread press/press/doctype/press_job/jobs/increase_disk_size.py
@tanmoysrt tanmoysrt force-pushed the move_press_job_to_code branch 5 times, most recently from f3e14fa to 5d7945e Compare April 24, 2026 10:39
@tanmoysrt tanmoysrt force-pushed the move_press_job_to_code branch from eda3467 to 7d47f5d Compare April 24, 2026 13:05
Copy link
Copy Markdown
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

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

Copilot encountered an error and was unable to review this pull request. You can try again by re-requesting a review.

@tanmoysrt tanmoysrt merged commit 2ece3fb into frappe:develop Apr 25, 2026
14 of 15 checks passed
@frappe-pr-bot
Copy link
Copy Markdown
Collaborator

🎉 This PR is included in version 0.20.0 🎉

The release is available on GitHub release

Your semantic-release bot 📦🚀

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Move Press Jobs to Code

3 participants