Skip to content

Background workers#83

Open
Yannicked wants to merge 41 commits into
iterorganization:developfrom
Yannicked:feature/celery-tasks
Open

Background workers#83
Yannicked wants to merge 41 commits into
iterorganization:developfrom
Yannicked:feature/celery-tasks

Conversation

@Yannicked

Copy link
Copy Markdown
Collaborator

This PR introduces simulation ingestion via Celery background workers.

When simulations are uploaded through the new v1.3 API endpoint, file copying and status tracking are now handled by Celery tasks (copy_files_task chained with complete_ingestion_task) rather than blocking the HTTP request.

The ingestion pipeline tracks status though the following: QUEUED → COPYING → COPIED → VALIDATING → VALIDATED → COMPLETED, with failure variants.

@Yannicked
Yannicked force-pushed the feature/celery-tasks branch from 06bf048 to 68938a1 Compare May 28, 2026 08:19
@Yannicked
Yannicked requested a review from ioan-alexandra June 2, 2026 14:28
@Yannicked Yannicked mentioned this pull request Jun 8, 2026
…3 chain

complete_ingestion_task opened a DB session via get_db() but never closed
it, leaking a connection on every ingestion (it is the terminal task of the
v1.3 chain). Wrap its body in try/finally so the session is always closed,
matching copy_files_task and validate_imas_task.

validate_imas_task was a stub that always set VALIDATED without validating,
and the v1.3 ingestion chain never ran it, so v1.3 silently ignored
validation.auto_validate / error_on_fail that v1/v1.1/v1.2 honor. The task
now runs the same schema Validator v1.2 uses, sets VALIDATION_FAILED and
(when error_on_fail) re-raises to abort the chain before COMPLETED. The v1.3
endpoint inserts the validation step into the chain when auto_validate is set.
to_model_with_path passed the SQLAlchemy URI TypeDecorator (imported from
.types) to sha1_checksum instead of a simdb.uri.URI, so sha1_checksum raised
AttributeError on uri.scheme for any IMAS file. Construct the URI via the
urilib.URI parser, matching from_data / from_data_model.
@Yannicked
Yannicked marked this pull request as ready for review June 24, 2026 11:30
@olivhoenen

Copy link
Copy Markdown
Contributor

Please fix conflicts @Yannicked

@SimonPinches SimonPinches left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

A few comments:

  • I think you need to make celery a server dependency in pyproject.toml (or guard the import of celery) since otherwise pip install imas-simdb[server] will produce a server that crashes on import.
  • If the broker is down when apply_async() is called (the row is already committed as QUEUED), or a worker dies mid-COPYING (tasks are not acks_late, so the message is gone), the simulation will get stuck in a non-terminal state forever, and delete_simulation will return 409. The only recovery would seem to be manual DB surgery so we should probably add either an admin force-delete capability or a staleness timeout.
  • The watcher/notification behaviour is changed. Now body.add_watcher is ignored and the send_email_task never used - is this deliberate?

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.

Create Updated Simulations Endpoint For New Upload architecture Implement Celery task queue

6 participants