Skip to content

Conversation

@suricactus
Copy link
Collaborator

This PR addressed multiple problems:

  • job definition requires changes in multiple files - entrypoint.py, custom file etc
  • extending QFieldCloud with custom jobs is impossible unless entrypoint.py is overwritten
  • testability of the qgis worker as a standalone module without going through the worker wrapper is cumbersome
  • some files (e.g. apply_deltas.py) defined two CLI interfaces to be called - one via entrypoint.py, and one by running the file itself.

@duke-nyuki
Copy link
Collaborator

Base automatically changed from QF-6714-libqfieldsync-followup to master September 23, 2025 22:50
@suricactus suricactus force-pushed the QF-6803-extendible-jobs branch 2 times, most recently from a41db4a to 04bf260 Compare September 24, 2025 00:44
@suricactus suricactus changed the base branch from master to bump_dependencies September 24, 2025 00:45
@suricactus suricactus changed the title feat(worker): make qgis jobs defined in a single file [WIP] feat(worker): make qgis jobs defined in a single file Sep 24, 2025
@suricactus suricactus force-pushed the QF-6803-extendible-jobs branch from e9c77f0 to 08d6500 Compare September 30, 2025 00:15
Base automatically changed from bump_dependencies to master October 1, 2025 11:07
@suricactus suricactus force-pushed the QF-6803-extendible-jobs branch 3 times, most recently from 8741d20 to f10e6a9 Compare October 1, 2025 19:24
The utils.py is way too complex with all the functionality in there.
Make sure the steps in a workflow are always unique.
Used by projects based on QFieldCloud but need to modify the existing
jobs.
Instead of having `QfcBaseWorkflow.handle` to take care of building the
workflow and making sure the `feedback.json` is generated, we make the
`handle` still prepare the `feedback.json`, but now in the base class
and the new method `get_workflow` must be implemented in the children.

This also allow for easier extension of already existing jobs.
it should always been `upload_project_directory` as there is no export
@suricactus suricactus force-pushed the QF-6803-extendible-jobs branch from f10e6a9 to 30c5e52 Compare October 1, 2025 19:24
return {
Job.Type.PACKAGE: PackageJobRun,
Job.Type.DELTA_APPLY: DeltaApplyJobRun,
Job.Type.DELTA_APPLY: ApplyDeltaJobRun,
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

I promise one day we also change the DELTA_APPLY name with APPLY_DELTA. I am sorry for the dislexic name I introduced back then.

Copy link
Contributor

Choose a reason for hiding this comment

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

Noted that down for the follow up task to make the Job.Type extensible (so that custom jobs are correctly displayed in the admin)

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Do you already have CU task? Can you post a link here?

@suricactus suricactus changed the title [WIP] feat(worker): make qgis jobs defined in a single file feat(worker): make qgis jobs defined in a single file Oct 2, 2025
@suricactus suricactus requested review from boardend and Copilot October 2, 2025 00:23
Copy link

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.

Pull Request Overview

This PR restructures the QGIS worker to consolidate job definitions into a single file architecture. It addresses the problem where job definitions required changes across multiple files and makes extending QFieldCloud with custom jobs easier.

  • Consolidates QGIS worker job definitions from multiple scattered files into a unified command-based architecture
  • Introduces a new workflow system that enables better testability and extensibility
  • Restructures entrypoint.py to use the new command system instead of hardcoded functions

Reviewed Changes

Copilot reviewed 17 out of 17 changed files in this pull request and generated no comments.

Show a summary per file
File Description
scripts/check_envvars.sh Updates environment variable exclusion list for new debug variable
docker-qgis/qfc_worker/workflow.py New workflow execution framework with step-based processing
docker-qgis/qfc_worker/utils.py Removes workflow classes moved to separate workflow.py file
docker-qgis/qfc_worker/exceptions.py New centralized exception definitions extracted from utils.py
docker-qgis/qfc_worker/commands_base.py New command registration system and base class for QGIS commands
docker-qgis/qfc_worker/commands/*.py Refactored job implementations using the new command architecture
docker-qgis/entrypoint.py Simplified to delegate to the new command system
docker-compose.override.local.yml Updates debug environment variables for new development setup
docker-app/worker_wrapper/wrapper.py Refactored container execution and debug mounting logic
docker-app/qfieldcloud/settings.py Consolidated debug path settings
docker-app/qfieldcloud/core/tests/test_delta.py Minor test improvements for better error reporting
docker-app/qfieldcloud/core/models.py Added explicit project_id type annotation
docker-app/qfieldcloud/core/management/commands/dequeue.py Updated job class references and improved structure
.env.example Updated environment variable documentation

Tip: Customize your code reviews with copilot-instructions.md. Create the file or learn how to get started.

@suricactus suricactus marked this pull request as ready for review October 2, 2025 00:24
@suricactus
Copy link
Collaborator Author

@boardend can you have a look on this PR this week or the next week latest?

@boardend
Copy link
Contributor

boardend commented Oct 9, 2025

Sure, will review and test over the weekend

Copy link
Contributor

@boardend boardend left a comment

Choose a reason for hiding this comment

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

LGTM 😍

return {
Job.Type.PACKAGE: PackageJobRun,
Job.Type.DELTA_APPLY: DeltaApplyJobRun,
Job.Type.DELTA_APPLY: ApplyDeltaJobRun,
Copy link
Contributor

Choose a reason for hiding this comment

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

Noted that down for the follow up task to make the Job.Type extensible (so that custom jobs are correctly displayed in the admin)

@suricactus suricactus merged commit 74dabaa into master Oct 15, 2025
21 checks passed
@suricactus suricactus deleted the QF-6803-extendible-jobs branch October 15, 2025 08:14
@suricactus
Copy link
Collaborator Author

@boardend thanks for the review!

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.

4 participants