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

Conversation

benoit74
Copy link
Collaborator

@benoit74 benoit74 commented Nov 9, 2023

Rationale

Fix #864 (again)
Fix #867 (mandatory to deploy #864 on youzim.it)

Changes

@benoit74 benoit74 self-assigned this Nov 9, 2023
@benoit74 benoit74 force-pushed the zimit_checks branch 2 times, most recently from 1239c8a to 3783f1b Compare November 9, 2023 09:47
@benoit74 benoit74 marked this pull request as ready for review November 9, 2023 09:52
@benoit74
Copy link
Collaborator Author

benoit74 commented Nov 9, 2023

Nota: do not merge until decision has been taken on openzim/zimit-frontend#42 + synchronization with Travis is needed

@benoit74 benoit74 requested a review from rgaudin November 10, 2023 09:52
Copy link
Member

@rgaudin rgaudin left a comment

Choose a reason for hiding this comment

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

Thank you ; please take a look at suggestions and question

dispatcher/backend/src/common/constants.py Outdated Show resolved Hide resolved
@benoit74
Copy link
Collaborator Author

benoit74 commented Nov 14, 2023

Following your review, I made following changes:

  • create a ZimitFlagsSchemaRelaxed class which override the zim_file field to remove the validation
  • use conditionally the ZimitFlagsSchemaRelaxed in get_offliner_schema based on environment variable
  • renamed the environment variable to ZIMIT_USE_RELAXED_SCHEMA to better match its "new" purpose + transformed into a bool
  • fixed tests so that all cases are now covered by manipulating the constant (manipulating the constant is where my bad Python programming made me fail previously)

@benoit74 benoit74 requested a review from rgaudin November 14, 2023 12:39
Copy link
Member

@rgaudin rgaudin left a comment

Choose a reason for hiding this comment

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

LGTM ; couple minor suggestions. I'll let you decide what to do. Please merge yourself

@benoit74 benoit74 marked this pull request as draft November 16, 2023 10:45
@benoit74
Copy link
Collaborator Author

Suggested changes made, makes a lot of sense.

I converted this PR to Draft so that it is not merged by accident, we need to synchronize with Travis + prepare new env variable for youzim.it

@benoit74 benoit74 marked this pull request as ready for review November 17, 2023 07:27
@benoit74 benoit74 merged commit e078943 into main Nov 17, 2023
5 checks passed
@benoit74 benoit74 deleted the zimit_checks branch November 17, 2023 07:27
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
2 participants