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

Check recipe flags at schedule creation (including fix for youzim.it) #868

Merged
merged 4 commits into from
Nov 17, 2023
Merged
Show file tree
Hide file tree
Changes from 1 commit
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion dispatcher/backend/src/common/constants.py
Original file line number Diff line number Diff line change
Expand Up @@ -99,4 +99,4 @@
REQ_TIMEOUT_GHCR = int(os.getenv("REQ_TIMEOUT_GHCR", 10))

# OFFLINERS
ZIMIT_DISABLE_ZIM_FILENAME_CHECK = os.getenv("ZIMIT_DISABLE_ZIM_FILENAME_CHECK", "")
ZIMIT_USE_RELAXED_SCHEMA = bool(os.getenv("ZIMIT_USE_RELAXED_SCHEMA"))
6 changes: 5 additions & 1 deletion dispatcher/backend/src/common/schemas/models.py
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,7 @@

from marshmallow import Schema, fields, pre_load, validate, validates_schema

from common import constants
from common.enum import DockerImageName, Offliner, Platform
from common.schemas import SerializableSchema, String
from common.schemas.fields import (
Expand Down Expand Up @@ -33,6 +34,7 @@
WikihowFlagsSchema,
YoutubeFlagsSchema,
ZimitFlagsSchema,
ZimitFlagsSchemaRelaxed,
)


Expand Down Expand Up @@ -84,7 +86,9 @@ def get_offliner_schema(offliner):
Offliner.nautilus: NautilusFlagsSchema,
Offliner.ted: TedFlagsSchema,
Offliner.openedx: OpenedxFlagsSchema,
Offliner.zimit: ZimitFlagsSchema,
Offliner.zimit: ZimitFlagsSchemaRelaxed
if constants.ZIMIT_USE_RELAXED_SCHEMA
else ZimitFlagsSchema,
Offliner.kolibri: KolibriFlagsSchema,
Offliner.wikihow: WikihowFlagsSchema,
Offliner.ifixit: IFixitFlagsSchema,
Expand Down
3 changes: 2 additions & 1 deletion dispatcher/backend/src/common/schemas/offliners/__init__.py
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,7 @@
from common.schemas.offliners.ted import TedFlagsSchema
from common.schemas.offliners.wikihow import WikihowFlagsSchema
from common.schemas.offliners.youtube import YoutubeFlagsSchema
from common.schemas.offliners.zimit import ZimitFlagsSchema
from common.schemas.offliners.zimit import ZimitFlagsSchema, ZimitFlagsSchemaRelaxed

__all__ = (
"FreeCodeCampFlagsSchema",
Expand All @@ -25,6 +25,7 @@
"WikihowFlagsSchema",
"YoutubeFlagsSchema",
"ZimitFlagsSchema",
"ZimitFlagsSchemaRelaxed",
)


Expand Down
21 changes: 17 additions & 4 deletions dispatcher/backend/src/common/schemas/offliners/zimit.py
Original file line number Diff line number Diff line change
@@ -1,6 +1,5 @@
from marshmallow import fields, validate

from common.constants import ZIMIT_DISABLE_ZIM_FILENAME_CHECK
from common.schemas import SerializableSchema, String, StringEnum
from common.schemas.fields import (
validate_output,
Expand Down Expand Up @@ -191,9 +190,7 @@ class Meta:
"Make sure to end with _{period}.zim",
},
data_key="zim-file",
validate=validate_zim_filename
if not ZIMIT_DISABLE_ZIM_FILENAME_CHECK
else None,
validate=validate_zim_filename,
)

tags = String(
Expand Down Expand Up @@ -471,3 +468,19 @@ class Meta:
data_key="adminEmail",
required=False,
)


class ZimitFlagsSchemaRelaxed(ZimitFlagsSchema):
"""A Zimit flags schema with relaxed constraints on validation

For now, only zim_file name is not checked anymore. Typically used for youzim.it
"""

zim_file = String(
metadata={
"label": "ZIM filename",
"description": "ZIM file name (based on --name if not provided). "
"Make sure to end with _{period}.zim",
},
data_key="zim-file",
)
Original file line number Diff line number Diff line change
@@ -1,83 +1,91 @@
class TestZimit:
def test_create_zimit_schedule_ok(self, client, access_token, garbage_collector):
schedule = {
"name": "zimit_test_ok",
"category": "other",
"enabled": False,
"tags": [],
"language": {"code": "fr", "name_en": "French", "name_native": "Français"},
"config": {
"task_name": "zimit",
"warehouse_path": "/other",
"image": {"name": "openzim/zimit", "tag": "1.0.0"},
"monitor": False,
"platform": None,
"flags": {
"name": "acme",
"url": "https://www.acme.com",
"zim-file": "acme_en_all_{period}.zim",
},
"resources": {"cpu": 3, "memory": 1024, "disk": 0},
},
"periodicity": "quarterly",
}
from dataclasses import dataclass
from typing import List

url = "/schedules/"
response = client.post(
url, json=schedule, headers={"Authorization": access_token}
)
response_data = response.get_json()
print(response_data)
if "_id" in response_data:
garbage_collector.add_schedule_id(response_data["_id"])
assert response.status_code == 201
import pytest

from common import constants

# below test is green only if ZIMIT_DISABLE_ZIM_FILENAME_CHECK is set
# def test_create_zimit_schedule_bad_name_ok(
# self, client, access_token, garbage_collector
# ):
# schedule = {
# "name": "zimit_test_bad_name_ok",
# "category": "other",
# "enabled": False,
# "tags": [],
# "language": {
# "code": "fr",
# "name_en": "French",
# "name_native": "Français"
# },
# "config": {
# "task_name": "zimit",
# "warehouse_path": "/other",
# "image": {"name": "openzim/zimit", "tag": "1.0.0"},
# "monitor": False,
# "platform": None,
# "flags": {
# "name": "acme",
# "url": "https://www.acme.com",
# "zim-file": "bad_name",
# },
# "resources": {"cpu": 3, "memory": 1024, "disk": 0},
# },
# "periodicity": "quarterly",
# }

# url = "/schedules/"
# response = client.post(
# url, json=schedule, headers={"Authorization": access_token}
# )
# response_data = response.get_json()
# print(response_data)
# if "_id" in response_data:
# garbage_collector.add_schedule_id(response_data["_id"])
# assert response.status_code == 201
def update_dict(dict: dict, key_path: str, new_value: any):
benoit74 marked this conversation as resolved.
Show resolved Hide resolved
# Split the key path into individual keys
keys = key_path.split(".")

# below test becomes red if ZIMIT_DISABLE_ZIM_FILENAME_CHECK is set
def test_create_zimit_schedule_bad_name_nok(
self, client, access_token, garbage_collector
# Initialize a reference to the nested dictionary
current_dict = dict

# Navigate through the nested structure
for key in keys[:-1]:
current_dict = current_dict[key]

# Update the value using the last key
current_dict[keys[-1]] = new_value


class TestZimit:
@dataclass
class Modification:
benoit74 marked this conversation as resolved.
Show resolved Hide resolved
key_path: str
new_value: str

@pytest.mark.parametrize(
"modifications, relaxed_schema, succeeds",
[
(
[
Modification(
key_path="name", new_value="zimit_test_good_name_not_relaxed"
)
],
False,
True,
),
(
[
Modification(
key_path="name", new_value="zimit_test_good_name_relaxed"
)
],
True,
True,
),
(
[
Modification(
key_path="name", new_value="zimit_test_bad_name_not_relaxed"
),
Modification(
key_path="config.flags.zim-file", new_value="bad_name"
),
],
False,
False,
),
(
[
Modification(
key_path="name", new_value="zimit_test_bad_name_relaxed"
),
Modification(
key_path="config.flags.zim-file", new_value="bad_name"
),
],
True,
True,
),
],
)
def test_create_zimit_schedule_generic(
self,
client,
access_token,
garbage_collector,
modifications: List[Modification],
relaxed_schema: bool,
succeeds: bool,
):
constants.ZIMIT_USE_RELAXED_SCHEMA = relaxed_schema
schedule = {
"name": "zimit_test_bad_name_nok",
"name": "zimit_test_ok",
"category": "other",
"enabled": False,
"tags": [],
Expand All @@ -91,21 +99,24 @@ def test_create_zimit_schedule_bad_name_nok(
"flags": {
"name": "acme",
"url": "https://www.acme.com",
"zim-file": "bad_name",
"zim-file": "acme_en_all_{period}.zim",
},
"resources": {"cpu": 3, "memory": 1024, "disk": 0},
},
"periodicity": "quarterly",
}

for modification in modifications:
update_dict(schedule, modification.key_path, modification.new_value)
url = "/schedules/"
response = client.post(
url, json=schedule, headers={"Authorization": access_token}
)
response_data = response.get_json()
print(response_data)
if "_id" in response_data:
garbage_collector.add_schedule_id(response_data["_id"])
assert response.status_code == 400
assert "error_description" in response_data
assert "zim-file" in response_data["error_description"]
if succeeds:
assert response.status_code == 201
else:
assert response.status_code == 400
assert "error_description" in response_data
assert "zim-file" in response_data["error_description"]