Skip to content

Add seed file support to Browsertrix backend #2710

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

Merged
merged 61 commits into from
Jul 23, 2025

Conversation

tw4l
Copy link
Member

@tw4l tw4l commented Jul 2, 2025

Fixes #2673

Changes in this PR:

  • Adds a new file_uploads.py module and corresponding /files API prefix with methods/endpoints for uploading, GETing, and deleting seed files (can be extended to other types of files moving forward)
  • Seed files are supported via CrawlConfig.config.seedFileId on POST and PATCH endpoints. This seedFileId is replaced by a presigned url when passed to the crawler by the operator
  • Seed files are read when first uploaded to calculate firstSeed and seedCount and store them in the database, and this is copied into the workflow and crawl documents when they are created.
  • Logic is added to store firstSeed and seedCount for other workflows as well, and a migration added to backfill data, to maintain consistency and fix some of the pymongo aggregations that previously assumed all workflows would have at least one Seed object in CrawlConfig.seeds
  • Seed file and thumbnail storage stats are added to org stats
  • Seed file and thumbnail uploads first check that the org's storage quota has not been exceeded and return a 400 if so
  • A cron background job (run weekly each Sunday at midnight by default, but configurable) is added to look for seed files at least x minutes old (1440 minutes, or 1 day, by default, but configurable) that are not in use in any workflows, and to delete them when they are found. The backend pods will ensure this k8s batch job exists when starting up and create it if it does not already exist. A database entry for each run of the job is created in the operator on job completion so that it'll appear in the /jobs API endpoints, but retrying of this type of regularly scheduled background job is not supported as we don't want to accidentally create multiple competing scheduled jobs.
  • Adds a min_seed_file_crawler_image value to the Helm chart that is checked before creating a crawl from a workflow if set. If a workflow cannot be run, return the detail of the exception in CrawlConfigAddedResponse.errorDetail so that we can display the reason in the frontend

## Todo

- Modify test chart crawler release back to latest once crawler with seed file support is properly released

@tw4l tw4l force-pushed the issue-2673-seed-file-backend-support branch from d6cecdb to 690238b Compare July 3, 2025 14:45
@tw4l tw4l force-pushed the issue-2673-seed-file-backend-support branch 6 times, most recently from 9741bac to dc9efcf Compare July 10, 2025 16:20
@tw4l tw4l changed the title Work in progress: Add seed file support to Browsertrix backend Add seed file support to Browsertrix backend Jul 10, 2025
@tw4l tw4l marked this pull request as ready for review July 10, 2025 17:19
@tw4l
Copy link
Member Author

tw4l commented Jul 10, 2025

Some of the all crawls tests toward the end of the test run in test_uploads.py are periodically returning 502, I'm not quite sure what's happening there but will keep looking into it. Otherwise this is now ready for review.

@tw4l tw4l requested review from ikreymer and SuaYoo July 10, 2025 17:20
@SuaYoo SuaYoo changed the base branch from main to issue-2673-seed-file July 14, 2025 19:41
@SuaYoo

This comment was marked as resolved.

@tw4l

This comment was marked as resolved.

@tw4l tw4l force-pushed the issue-2673-seed-file-backend-support branch 2 times, most recently from a31fceb to 69a5c5a Compare July 16, 2025 15:22
Copy link
Member

@SuaYoo SuaYoo left a comment

Choose a reason for hiding this comment

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

Reviewing functionality of API changes against frontend-upload-seed-url-list, working well! I didn't review code or other functionality.

@tw4l tw4l marked this pull request as draft July 16, 2025 21:56
@tw4l
Copy link
Member Author

tw4l commented Jul 16, 2025

Converted to draft while making sure the cron job to clean up unused seed files is working as expected. Getting close but there seems to be a permissions issue preventing the batch job from being created in the default k8s namespace:

future: <Task finished name='Task-4' coro=<BackgroundJobOps.ensure_cron_cleanup_jobs_exist() done, defined at /app/btrixcloud/background_jobs.py:478> exception=FailToCreateError([ApiException()])>
Traceback (most recent call last):
  File "/app/btrixcloud/background_jobs.py", line 480, in ensure_cron_cleanup_jobs_exist
    await self.crawl_manager.ensure_cleanup_seed_file_cron_job_exists()
  File "/app/btrixcloud/crawlmanager.py", line 257, in ensure_cleanup_seed_file_cron_job_exists
    await self.create_from_yaml(data, namespace=DEFAULT_NAMESPACE)
  File "/app/btrixcloud/k8sapi.py", line 190, in create_from_yaml
    created = await create_from_dict(
              ^^^^^^^^^^^^^^^^^^^^^^^
  File "/usr/local/lib/python3.12/site-packages/kubernetes_asyncio/utils/create_from_yaml.py", line 148, in create_from_dict
    raise FailToCreateError(api_exceptions)
kubernetes_asyncio.utils.create_from_yaml.FailToCreateError: Error from server (Forbidden): {"kind":"Status","apiVersion":"v1","metadata":{},"status":"Failure","message":"cronjobs.batch is forbidden: User \"system:serviceaccount:default:default\" cannot create resource \"cronjobs\" in API group \"batch\" in the namespace \"default\"","reason":"Forbidden","details":{"group":"batch","kind":"cronjobs"},"code":403}

The job was previously in the crawlers namespace, but that meant it didn't have access to secrets it needed e.g. for database access.

Will continue to work on this tomorrow unless @ikreymer beats me to the punch

Edit: resolved

@tw4l tw4l force-pushed the issue-2673-seed-file-backend-support branch from 5031352 to ac499f4 Compare July 17, 2025 15:13
@tw4l tw4l marked this pull request as ready for review July 17, 2025 15:47
@tw4l
Copy link
Member Author

tw4l commented Jul 17, 2025

@ikreymer Seed file cleanup is now working, tested locally, passing in CI, and more configurable via helm chart values. Ready for your eyes again.

@tw4l tw4l force-pushed the issue-2673-seed-file-backend-support branch from b321e35 to 34efaeb Compare July 17, 2025 16:07
tw4l and others added 24 commits July 22, 2025 18:36
Also makes delta for how old unused files need to be before
getting cleaned up configurable via helm chart so that we
can set it to 1 minute for nightly tests.
This is probably a step forward but may still be confusing until
we rework collection thumbnails to be stored in the new files db
collection as well and rationalize into one set of models.
… either to current origin or to internal origin, if no headers provided

- use resolve_relative_access_path() for both file uploads and collection thumbnails
- consolidate UserUploadFile into SeedFile, can split again if needed
- add get_absolute_presigned_url() to UserFile for convenience in getting absolute url directly
@ikreymer ikreymer force-pushed the issue-2673-seed-file-backend-support branch from a0d3679 to 1b6b4e0 Compare July 23, 2025 01:36
@ikreymer ikreymer merged commit 3d38669 into issue-2673-seed-file Jul 23, 2025
22 checks passed
@ikreymer ikreymer deleted the issue-2673-seed-file-backend-support branch July 23, 2025 02:07
ikreymer added a commit that referenced this pull request Jul 23, 2025
Fixes #2673 

Changes in this PR:

- Adds a new `file_uploads.py` module and corresponding `/files` API
prefix with methods/endpoints for uploading, GETing, and deleting seed
files (can be extended to other types of files moving forward)
- Seed files are supported via `CrawlConfig.config.seedFileId` on POST
and PATCH endpoints. This seedFileId is replaced by a presigned url when
passed to the crawler by the operator
- Seed files are read when first uploaded to calculate `firstSeed` and
`seedCount` and store them in the database, and this is copied into the
workflow and crawl documents when they are created.
- Logic is added to store `firstSeed` and `seedCount` for other
workflows as well, and a migration added to backfill data, to maintain
consistency and fix some of the pymongo aggregations that previously
assumed all workflows would have at least one `Seed` object in
`CrawlConfig.seeds`
- Seed file and thumbnail storage stats are added to org stats
- Seed file and thumbnail uploads first check that the org's storage
quota has not been exceeded and return a 400 if so
- A cron background job (run weekly each Sunday at midnight by default,
but configurable) is added to look for seed files at least x minutes old
(1440 minutes, or 1 day, by default, but configurable) that are not in
use in any workflows, and to delete them when they are found. The
backend pods will ensure this k8s batch job exists when starting up and
create it if it does not already exist. A database entry for each run of
the job is created in the operator on job completion so that it'll
appear in the `/jobs` API endpoints, but retrying of this type of
regularly scheduled background job is not supported as we don't want to
accidentally create multiple competing scheduled jobs.
- Adds a `min_seed_file_crawler_image` value to the Helm chart that is
checked before creating a crawl from a workflow if set. If a workflow
cannot be run, return the detail of the exception in
`CrawlConfigAddedResponse.errorDetail` so that we can display the reason
in the frontend
- Add SeedFile model from base UserFile (former ImageFIle), ensure all APIs
returning uploaded files return an absolute pre-signed URL (either with external origin or internal service origin)

---------
Co-authored-by: Ilya Kreymer <[email protected]>
ikreymer added a commit that referenced this pull request Jul 23, 2025
Resolves #2646
Depends on #2710

## Changes

(Copied from #2689)

- Allows users to specify URL list as file.
- Allow uploading a text file of URLs
- Allow specifying >100 URLs into URL list, where they will turn into an uploaded list automatically.


---------
Co-authored-by: sua yoo <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

[Task]: Backend support for seed files
3 participants