upload zim to collection and create zimfarm task

[elfkuzco]

Rationale

This PR enhances the CMS by allowing editors to upload an already created zim from their local filesystem. The CMS on receiving such files creates the task on zimfarm which will advertise the book back to the CMS once checks and processing are done by a worker

Changes

• allow users to upload zims manually. uploads are done as S3 multipart uploads and API generates presigend URLs for uploading and executing the task. Presigned URLs for uploading expire after one day whereas the ones used by the worker expire after 7 days.
• add logic to authenticate with zimfarm API
• add task_id to book and corresponding UI link to view task on Zimfarm
• show tasks in collections views (where they were uploaded from)
• add shuttle task to delete zims of completed tasks
• add mill task to update status of requested tasks

This closes #322

[elfkuzco]

@benoit74 , if PR is approved, we will need to set policies on Wasabi to delete aborted/incomplete uploads and possibly retention policy on bucket too.

[elfkuzco]

Because uploads are done by editors, I have opted to keep the upload section in the collections page so collection_editor and global_editor can upload zims. But i'm stuck on the issue when book arrives and there's no title for it in which case collection_editor will not be able to act on it. Even worrying is that global editor when they try to promote book might attach book to a collection that is different from what the uploader intended. How do we go about this? Do we want to enhance promote book logic to only allow edits to the collection title when none is set? This way, when we determine book came from an uploaded zim, then, we fix it to that collection and path

@benoit74 , wdyt?

[codecov]

Codecov Report

❌ Patch coverage is 76.98925% with 107 lines in your changes missing coverage. Please review.
✅ Project coverage is 80.90%. Comparing base (380f7bd) to head (3ed6076).
⚠️ Report is 12 commits behind head on main.

Files with missing lines	Patch %	Lines
backend/src/cms_backend/utils/s3.py	44.44%	21 Missing and 9 partials ⚠️
backend/src/cms_backend/api/token.py	40.00%	24 Missing and 3 partials ⚠️
backend/src/cms_backend/api/routes/titles.py	75.32%	16 Missing and 3 partials ⚠️
backend/src/cms_backend/utils/requests.py	42.85%	8 Missing ⚠️
backend/src/cms_backend/db/book_actions.py	36.36%	6 Missing and 1 partial ⚠️
backend/src/cms_backend/mill/processors/book.py	44.44%	4 Missing and 1 partial ⚠️
...src/cms_backend/mill/update_zimfarm_task_status.py	91.17%	2 Missing and 1 partial ⚠️
backend/src/cms_backend/__init__.py	75.00%	1 Missing and 1 partial ⚠️
backend/src/cms_backend/db/requested_task.py	94.87%	2 Missing ⚠️
backend/src/cms_backend/api/context.py	94.44%	0 Missing and 1 partial ⚠️
... and 3 more

Additional details and impacted files

@@            Coverage Diff             @@
##             main     #469      +/-   ##
==========================================
- Coverage   81.13%   80.90%   -0.23%     
==========================================
  Files          64       69       +5     
  Lines        3881     4290     +409     
  Branches      473      507      +34     
==========================================
+ Hits         3149     3471     +322     
- Misses        581      657      +76     
- Partials      151      162      +11     

☔ View full report in Codecov by Harness.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:

• ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
• 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

[benoit74]

Because uploads are done by editors, I have opted to keep the upload section in the collections page so collection_editor and global_editor can upload zims. But i'm stuck on the issue when book arrives and there's no title for it in which case collection_editor will not be able to act on it. Even worrying is that global editor when they try to promote book might attach book to a collection that is different from what the uploader intended. How do we go about this? Do we want to enhance promote book logic to only allow edits to the collection title when none is set? This way, when we determine book came from an uploaded zim, then, we fix it to that collection and path

Good point ; can you develop what you mean by "enhance promote book logic to only allow edits to the collection title when none is set"?

[elfkuzco]

Here's the UI when API sets collection titles in create_title action

[elfkuzco]

I realized we need the recipe_id in order to be able to reconstruct this path. So, I think maybe we shouldn't delete recipe till zimfarm has published book to CMS or mill has detected task has failed/canceled.

[kelson42]

I realized we need the recipe_id in order to be able to reconstruct this path. So, I think maybe we shouldn't delete recipe till zimfarm has published book to CMS or mill has detected task has failed/canceled.

sorry to intervene here as @benoit74 is out for a few days, but here there onviously no recipe-id to link to the book/title.

[elfkuzco]

sorry to intervene here as @benoit74 is out for a few days, but here there onviously no recipe-id to link to the book/title.

Zimfarm advertises the recipe_id as part of the payload that gets sent in the CMS notification. And before we create the task on zimfarm, we create a recipe and store the recipe_id. But, I'm proposing to delay the deletion of the recipe on zimfarm (which is currently done immediately after creation) till it gets advertised to CMS.

[benoit74]

I did not had time to go through all details, but I have some preliminary feedback.

First, I'm now convinced we should have a zimfarm in the CMS docker-compose, just like we have one in WP1 ; probably mostly all components to test from end-to-end ; it is just too tedious to test the integration without it, too many bits to move to make it works ; please add it to the PR and ensure docker-compose default env values are correct to target this local CMS-tied Zimfarm.

Second, I feel like there is a conception issue. Reading again the issue notes, I realize that my last idea was to have the "Upload" function in the "TitleDetail" not in "CollectionDetail". Uploading at collection level adds just too much complexity. Not sure why you implemented it at that level. The problem is that we can be pretty sure that uploaded ZIM name has a significant chance to not match the title name we wanna use. We hence cannot say that the ZIM will follow the standard process, and we need to tie the upload to the target title. The user hence upload a ZIM to a given title and we "force" it to match. We should still be complaining when ZIM names does not match title name, but the book is automatically attached to proper title when it arrives.

WDYT? Do I miss something obvious we've already discussed?

Finally, regarding the fact that recipe will have been deleted when book arrives in the CMS, let's change our strategy and follow WP1 path: I realize we should never delete the Zimfarm recipe and reuse it over and over (creating it on first upload, and then only changing the URL each time a ZIM is uploaded). This gives the same benefits as WP1: full title history in Zimfarm in a single recipe. And also in this case better and simpler link between Zimfarm and CMS.

[elfkuzco]

Second, I feel like there is a conception issue. Reading again the issue notes, I realize that my last idea was to have the "Upload" function in the "TitleDetail" not in "CollectionDetail". Uploading at collection level adds just too much complexity. Not sure why you implemented it at that level. The problem is that we can be pretty sure that uploaded ZIM name has a significant chance to not match the title name we wanna use. We hence cannot say that the ZIM will follow the standard process, and we need to tie the upload to the target title. The user hence upload a ZIM to a given title and we "force" it to match. We should still be complaining when ZIM names does not match title name, but the book is automatically attached to proper title when it arrives.

WDYT? Do I miss something obvious we've already discussed?

Rationale for keeping it in the collection detail view is that collection editors do not have permission to create a title. So, if we keep it in the title detail view, this would be based on the assumption the title already exists. But that would mean, collection editor cannot upload zim because they cannot create title. Also, I was assuming a title can belong to many collections (via multiple collection title). Which collection permissions do we want to use assuming one is private and one is public? If we take the public route and zim arrives, then all collection editors can see the book that is pending association to a title.

[benoit74]

Rationale for keeping it in the collection detail view is that collection editors do not have permission to create a title. So, if we keep it in the title detail view, this would be based on the assumption the title already exists. But that would mean, collection editor cannot upload zim because they cannot create title.
Also, I was assuming a title can belong to many collections (via multiple collection title). Which collection permissions do we want to use assuming one is private and one is public? If we take the public route and zim arrives, then all collection editors can see the book that is pending association to a title.

Given current usage, I feel like it would make more sense to have global editors create the title, assign it to proper collection and path (because they are the ones in the know of these details, especially the fact that title name being unique across all collections, choosing it wisely and ensuring it matches our convention is very important), and then tell the person who wants to upload a ZIM manually "hey, I have this title ready for you to upload ZIMs to, please proceed". Also the fact that collection editor cannot create a title should not be "bypassed" by the ability to upload a ZIM manually which would create a title automatically for them. And this limitation is not "written in stone" IMHO, this might change in the future.

Regarding titles associated to both a public and a private collection, we know the title and its books are in fact public, because public wins over private ATM.

I don't get the point about book pending association to a title. A book pending association to a title is normally hidden to everybody but global-editors and admins. And with what I propose, ZIMs manually uploaded must anyway have a strong link to the title they have been uploaded to, because I'm quite sure we will have bad ZIM names in books arriving, so we should not trust the usual ZIM name <=> title name association, it will break too often.

[elfkuzco]

Finally, regarding the fact that recipe will have been deleted when book arrives in the CMS, let's change our strategy and follow WP1 path: I realize we should never delete the Zimfarm recipe and reuse it over and over (creating it on first upload, and then only changing the URL each time a ZIM is uploaded). This gives the same benefits as WP1: full title history in Zimfarm in a single recipe. And also in this case better and simpler link between Zimfarm and CMS.

I noticed we couldn't request another task while current one was being requested. Given the uploads are now in the title view, I have constructed the recipe names as zimwright_{title.name} (maybe I should have used title ID instead). This way, users can upload to other title details.

In logic to promote book dialog, should we use the name from zim metadata or the one from which the zim was uploaded?

[elfkuzco]

I also realized that during the integration of zimfarm components in cms, worker running and zimfarm advertising back to cms would cause shuttle to fail because both don't share the same local paths. i was thinking of worker uploading zim to s3 and shuttle proceeding as usual. but our current shuttle implementation works only with local files. do we want to extend shuttle codebase to work with s3 too?

[benoit74]

I also realized that during the integration of zimfarm components in cms, worker running and zimfarm advertising back to cms would cause shuttle to fail because both don't share the same local paths.

They must share the same local path / volume ; or more exactly, the folder where zimfarm receiver places received files must be seen by CMS shuttle as the quarantine folder. This is how things work in production. This has to be replicated in docker-compose obviously.

i was thinking of worker uploading zim to s3 and shuttle proceeding as usual. but our current shuttle implementation works only with local files. do we want to extend shuttle codebase to work with s3 too?

We might need to do that at some point, but I don't see a reason for not been able to replicate production setup in dev docker-compose ... and I want dev docker-compose to replicate production setup, the less different from production it is, the more bugs we will catch (or the less unrelated bugs we will experience while testing).

I noticed we couldn't request another task while current one was being requested. Given the uploads are now in the title view, I have constructed the recipe names as zimwright_{title.name} (maybe I should have used title ID instead). This way, users can upload to other title details.

We indeed need to have one recipe per title.

In logic to promote book dialog, should we use the name from zim metadata or the one from which the zim was uploaded?

When arriving in the CMS, the book must automatically be associated with the title the ZIM was uploaded to. So in DB book.title_id will be set. Which means mill.processors.book.get_matching_title() will have to be modified to automatically match the book to proper title. Once book is matched to proper title, does CMS really still check the name? Which action is raised in promote book dialog and is it a mandatory or an optional one?

When ZIM name does not match title name (but book has been associated to a title because it has been uploaded to it), I feel like the CMS should raise a "Metadata Mismatch", and then an optional action to update title name.

[elfkuzco]

I have updated the compose file to integrate receiver and bound the receiver volumes to the shuttle path. complete interaction between cms and zimfarm now works flawlessly.

[elfkuzco]

. Once book is matched to proper title, does CMS really still check the name?

No, CMS doesn't check it's name once book has been attached to title.

Which action is raised in promote book dialog and is it a mandatory or an optional one?

No action is suggested if there's a mismatch between title name and book name.

[benoit74]

Thank you!

Why did you kept the tasks tab in the collection details view? I feel like it would be way more useful in the title details, next to "upload". I would even suggest we have only one tab "Upload" with all information inside ; both are very closely related, and we have quite a lot of space left in both tabs.

Why did you keep asking for collection path in "Upload"? I don't think this is necessary at all.

I had a CORS problem in dev stack, looks like Zimfarm API is not allowing CMS UI (to fetch offliners among others) ; I also don't get why it allows CMS API ; probably a port mismatch

I tried to upload a ZIM and it worked pretty well up to the point where Zimfarm is trying to upload. I realized we miss the DISABLE_WAREHOUSE_PATH: true setting in Zimfarm API and in Zimfarm UI.

We can also revisit the message when upload is complete:

We should better indicate that upload will now be processed by the Zimfarm and CMS, and that user should watch book to arrive in title.

Requested tasks seem to never be updated, the function is not registered in mill/main.py. Default interval should be 1 min ; and I don't feel like building a list of requested task ids to omit makes any sense. The list will only keep growing, up to the point it will not be tractable by DB engine anymore. Why not simply passing list of statuses to omit?

CMS also failed to match the incoming book with proper title, it is stuck in quarantine "pending title".

We also need to document what needs to be setup on Wasabi S3 bucket for upload to work correctly. I'm playing with configs to confirm that aspect.

[elfkuzco]

CMS also failed to match the incoming book with proper title, it is stuck in quarantine "pending title".

Applied this midnight but it should be working now. Also, there's been a change in the migrations, so you might want to downgrade and upgrade again. Applied all the other fixes.

I noticed that because of this logic

cms/backend/src/cms_backend/db/book.py

Lines 1341 to 1346 in beaabad

	# Update title name should it have changed (e.g. stackexchange domain updated
	# leading to ZIM name automatically updated as well)
	if title.name != book.name:
	title.events.append(f"{getnow()}: updating title name to {book.name}")
	title.name = book.name

when we associate the book to the title because it came from a requested task, it's name attempts to be updated and we can have an integrity error. Not sure how often that would come up in prod but it's worth mentioning.