-
-
Notifications
You must be signed in to change notification settings - Fork 29
remove model queries in alembic migration #1422
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
base: main
Are you sure you want to change the base?
Conversation
@benoit74 , I am going to need your input on the permissions required for the offliner creation via API. I avoided using WDYT? |
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## main #1422 +/- ##
==========================================
+ Coverage 82.37% 82.57% +0.20%
==========================================
Files 78 78
Lines 3863 3873 +10
Branches 423 424 +1
==========================================
+ Hits 3182 3198 +16
+ Misses 565 560 -5
+ Partials 116 115 -1 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thank you!
- Regarding permissions, yes we need a new permission to create/update offliner definitions
- I don't like the idea of getting a default offliner definition if version is not passed; this is very convenient in dev but very risky in production ; I feel like the version should be a mandatory parameter once WP1 and zimit-frontend have been migrated to pass this parameter ; and in this PR we assume it is
WP1 does not really have a UI to "select the version" they want to run and I figured they would also want to do similar thing. Because the API returns the versions as a list of strings in descending order of created_at. So, if they are going to select the version they would use by "choosing the first" because it should be the most recent, I figured it would be simpler to do it here and avoid one more API call. Same thing applies to Zimit frontend Do you have an idea on how the selection should be done? |
You probably saw that after your comment, just for the reference: openzim/wp1#1024 (comment) (we will configure the version in WP1 config file) |
1983ac5
to
38342ba
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM, please do not merge before openzim/zimit-frontend#176 and openzim/wp1#1024 are merged / solved
Moving to draft to avoid accident ^^
Changes
GET /offliners/{offliner_id}
endpointversion
optional while creating a schedule. attempt to select most recent version of the offliner definitionbackend-tools
from compose (don't think it really serves anymore)dev/README
to get any new dev started with new DB and updated instructionsThis closes #1391