From c3f1fb369b24efa5a5abcff1c78e7b2bd041ca66 Mon Sep 17 00:00:00 2001 From: Uchechukwu Orji Date: Wed, 15 Oct 2025 15:59:49 +0100 Subject: [PATCH 1/3] remove model queries in alembic migration --- .../zimfarm_backend/routes/offliners/logic.py | 39 +++++++++---------- .../zimfarm_backend/routes/schedules/logic.py | 18 +++++++-- .../routes/schedules/models.py | 4 +- dev/contrib/create_worker.sh | 2 +- 4 files changed, 38 insertions(+), 25 deletions(-) diff --git a/backend/src/zimfarm_backend/routes/offliners/logic.py b/backend/src/zimfarm_backend/routes/offliners/logic.py index 76b2e7f0e..1dea3de79 100644 --- a/backend/src/zimfarm_backend/routes/offliners/logic.py +++ b/backend/src/zimfarm_backend/routes/offliners/logic.py @@ -57,30 +57,29 @@ async def get_offliners( ) -@router.get("/{offliner_id}") -async def get_initial_offliner_version( - offliner_id: Annotated[str, Path()], +@router.post("") +async def create_offliner( + request: OfflinerCreateSchema, session: Annotated[OrmSession, Depends(gen_dbsession)], -) -> JSONResponse: - """Get a specific offliner""" + current_user: User = Depends(get_current_user), +) -> Response: + """Create an offliner""" + # TODO: Should we define new permissions for the current user that allows them to + # register new offliners? Because existing permissions overlap and it might not be + # feasible for someone who can create a schedule to create an offliner. + if not (current_user.scope and get_role_for(current_user.scope) == RoleEnum.ADMIN): + raise UnauthorizedError("You do not have permissions to create an offliner.") - # find the schema class that matches the offliner - offliner = db_get_offliner(session, offliner_id) - offliner_definition = db_get_offliner_definition( - session, offliner_id=offliner_id, version="initial" + db_create_offliner( + session, + offliner_id=request.offliner_id, + base_model=request.base_model, + docker_image_name=request.docker_image_name, + command_name=request.command_name, + ci_secret_hash=generate_password_hash(request.ci_secret_hash), ) - schema_cls = build_offliner_model(offliner, offliner_definition.schema_) - - flags = schema_to_flags(schema_cls) - return JSONResponse( - content={ - "flags": [flag.model_dump(mode="json", by_alias=True) for flag in flags], - "help": ( # dynamic + sourced from backend because it might be custom - f"https://github.com/openzim/{offliner}/wiki/Frequently-Asked-Questions" - ), - } - ) + return Response(status_code=HTTPStatus.CREATED) @router.post("") diff --git a/backend/src/zimfarm_backend/routes/schedules/logic.py b/backend/src/zimfarm_backend/routes/schedules/logic.py index dcaa73043..868a04cf7 100644 --- a/backend/src/zimfarm_backend/routes/schedules/logic.py +++ b/backend/src/zimfarm_backend/routes/schedules/logic.py @@ -43,6 +43,7 @@ create_offliner_instance, get_offliner_definition, get_offliner_definition_by_id, + get_offliner_versions, ) from zimfarm_backend.db.schedule import create_schedule as db_create_schedule from zimfarm_backend.db.schedule import ( @@ -134,9 +135,20 @@ def create_schedule( raise UnauthorizedError("You are not allowed to create a schedule") if offliner_id := request.config.get("offliner", {}).get("offliner_id"): - offliner_definition = get_offliner_definition( - session, offliner_id, request.version - ) + if request.version: + offliner_definition = get_offliner_definition( + session, offliner_id, request.version + ) + else: + # get the most recent offliner definition + results = get_offliner_versions(session, offliner_id, limit=1, skip=0) + if not results.versions: + raise RecordDoesNotExistError( + f"No offliner definitions found for {offliner_id}" + ) + offliner_definition = get_offliner_definition( + session, offliner_id, results.versions[0] + ) else: raise RequestValidationError( [ diff --git a/backend/src/zimfarm_backend/routes/schedules/models.py b/backend/src/zimfarm_backend/routes/schedules/models.py index 964578d3b..c4e803911 100644 --- a/backend/src/zimfarm_backend/routes/schedules/models.py +++ b/backend/src/zimfarm_backend/routes/schedules/models.py @@ -41,7 +41,9 @@ class ScheduleCreateSchema(BaseModel): periodicity: SchedulePeriodicity tags: list[NotEmptyString] = Field(default_factory=list) enabled: bool - version: str = "initial" # version of offliner to use for validation + # version of offliner to use for validation. Determine latest version to use + # if None is provided + version: str | None = None config: dict[str, Any] # will be validated in the route notification: ScheduleNotificationSchema | None = None context: NotEmptyString | None = None diff --git a/dev/contrib/create_worker.sh b/dev/contrib/create_worker.sh index 14689e0b2..9f039421f 100755 --- a/dev/contrib/create_worker.sh +++ b/dev/contrib/create_worker.sh @@ -17,7 +17,7 @@ ZF_ADMIN_TOKEN="$(curl -s -X 'POST' \ 'http://localhost:8000/v2/auth/authorize' \ -H 'accept: application/json' \ -H 'Content-Type: application/json' \ - -d '{"username": "admin", "password": "admin"}' \ + -d '{"username": "admin", "password": "admin_pass"}' \ | jq -r '.access_token')" echo "Create test_worker user" From 6f2f554a0c3bd496a3be7654ee760b4930ec728e Mon Sep 17 00:00:00 2001 From: Uchechukwu Orji Date: Wed, 15 Oct 2025 18:03:00 +0100 Subject: [PATCH 2/3] update dev/README and backend tools --- dev/contrib/create_worker.sh | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/dev/contrib/create_worker.sh b/dev/contrib/create_worker.sh index 9f039421f..14689e0b2 100755 --- a/dev/contrib/create_worker.sh +++ b/dev/contrib/create_worker.sh @@ -17,7 +17,7 @@ ZF_ADMIN_TOKEN="$(curl -s -X 'POST' \ 'http://localhost:8000/v2/auth/authorize' \ -H 'accept: application/json' \ -H 'Content-Type: application/json' \ - -d '{"username": "admin", "password": "admin_pass"}' \ + -d '{"username": "admin", "password": "admin"}' \ | jq -r '.access_token')" echo "Create test_worker user" From 1e46bcc8fe52cdf54bda9f62f79c7419a7aafb66 Mon Sep 17 00:00:00 2001 From: Uchechukwu Orji Date: Thu, 16 Oct 2025 11:21:44 +0100 Subject: [PATCH 3/3] require definition version while creating schedule --- .../zimfarm_backend/routes/schedules/logic.py | 18 +++--------------- .../zimfarm_backend/routes/schedules/models.py | 4 +--- backend/tests/routes/test_schedules.py | 4 ++++ 3 files changed, 8 insertions(+), 18 deletions(-) diff --git a/backend/src/zimfarm_backend/routes/schedules/logic.py b/backend/src/zimfarm_backend/routes/schedules/logic.py index 868a04cf7..dcaa73043 100644 --- a/backend/src/zimfarm_backend/routes/schedules/logic.py +++ b/backend/src/zimfarm_backend/routes/schedules/logic.py @@ -43,7 +43,6 @@ create_offliner_instance, get_offliner_definition, get_offliner_definition_by_id, - get_offliner_versions, ) from zimfarm_backend.db.schedule import create_schedule as db_create_schedule from zimfarm_backend.db.schedule import ( @@ -135,20 +134,9 @@ def create_schedule( raise UnauthorizedError("You are not allowed to create a schedule") if offliner_id := request.config.get("offliner", {}).get("offliner_id"): - if request.version: - offliner_definition = get_offliner_definition( - session, offliner_id, request.version - ) - else: - # get the most recent offliner definition - results = get_offliner_versions(session, offliner_id, limit=1, skip=0) - if not results.versions: - raise RecordDoesNotExistError( - f"No offliner definitions found for {offliner_id}" - ) - offliner_definition = get_offliner_definition( - session, offliner_id, results.versions[0] - ) + offliner_definition = get_offliner_definition( + session, offliner_id, request.version + ) else: raise RequestValidationError( [ diff --git a/backend/src/zimfarm_backend/routes/schedules/models.py b/backend/src/zimfarm_backend/routes/schedules/models.py index c4e803911..4f86b173e 100644 --- a/backend/src/zimfarm_backend/routes/schedules/models.py +++ b/backend/src/zimfarm_backend/routes/schedules/models.py @@ -41,9 +41,7 @@ class ScheduleCreateSchema(BaseModel): periodicity: SchedulePeriodicity tags: list[NotEmptyString] = Field(default_factory=list) enabled: bool - # version of offliner to use for validation. Determine latest version to use - # if None is provided - version: str | None = None + version: NotEmptyString config: dict[str, Any] # will be validated in the route notification: ScheduleNotificationSchema | None = None context: NotEmptyString | None = None diff --git a/backend/tests/routes/test_schedules.py b/backend/tests/routes/test_schedules.py index f8049d0ab..48128742a 100644 --- a/backend/tests/routes/test_schedules.py +++ b/backend/tests/routes/test_schedules.py @@ -168,6 +168,7 @@ def test_get_similar_schedules( }, "enabled": True, "periodicity": SchedulePeriodicity.manually.value, + "version": "initial", }, HTTPStatus.UNPROCESSABLE_ENTITY, id="invalid-config-missing-offliner-id", @@ -200,6 +201,7 @@ def test_get_similar_schedules( "monitor": True, }, "enabled": True, + "version": "initial", "periodicity": SchedulePeriodicity.manually.value, }, HTTPStatus.UNPROCESSABLE_ENTITY, @@ -266,6 +268,7 @@ def test_get_similar_schedules( }, "enabled": True, "periodicity": SchedulePeriodicity.manually.value, + "version": "initial", }, HTTPStatus.OK, id="valid-config", @@ -867,6 +870,7 @@ def test_create_duplicate_schedule( ), "enabled": True, "periodicity": SchedulePeriodicity.manually.value, + "version": "initial", }, ) assert response.status_code == HTTPStatus.CONFLICT