-
Notifications
You must be signed in to change notification settings - Fork 32
🎨 Store and retrieve task_name when listing Celery tasks
#7538
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
🎨 Store and retrieve task_name when listing Celery tasks
#7538
Conversation
Codecov ReportAttention: Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## master #7538 +/- ##
==========================================
+ Coverage 87.54% 88.89% +1.35%
==========================================
Files 1747 1400 -347
Lines 67652 57725 -9927
Branches 1146 485 -661
==========================================
- Hits 59224 51313 -7911
+ Misses 8107 6280 -1827
+ Partials 321 132 -189
Continue to review full report in Codecov by Sentry.
🚀 New features to boost your workflow:
|
services/storage/src/simcore_service_storage/modules/celery/backends/_redis.py
Outdated
Show resolved
Hide resolved
…arloromeo/osparc-simcore into is7528/add-name-when-listing-tasks
…arloromeo/osparc-simcore into is7528/add-name-when-listing-tasks
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.
Copilot reviewed 15 out of 15 changed files in this pull request and generated no comments.
Comments suppressed due to low confidence (3)
services/storage/src/simcore_service_storage/modules/celery/client.py:40
- [nitpick] The renaming from 'send_task' to 'submit_task' improves clarity; please double-check that all call sites have been updated accordingly.
async def submit_task(
services/storage/src/simcore_service_storage/modules/celery/models.py:48
- [nitpick] The legacy name validator in TaskMetadata sets a default empty 'name'; consider if an empty string is the most appropriate default for legacy tasks.
@model_validator(mode="before")
services/storage/src/simcore_service_storage/modules/celery/backends/_redis.py:81
- Verify that the ordering of keys from scan_iter aligns with the pipeline execution order to prevent any misalignment between keys and their corresponding metadata in the tasks list.
async for key in self._redis_client_sdk.redis.scan_iter(match=search_key + "*", count=_CELERY_TASK_SCAN_COUNT_PER_BATCH):
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.
Pull Request Overview
This PR enhances the storage service by adding support for storing and retrieving the task (job) name when listing Celery tasks. Key changes include:
- Updating the API and AsyncJobGet model to include a job_name field.
- Renaming methods (e.g. send_task to submit_task, set_progress to set_task_progress) and propagating these changes across tests and worker modules.
- Adapting tests and RPC endpoints to account for the new task/job name handling.
Reviewed Changes
Copilot reviewed 15 out of 15 changed files in this pull request and generated no comments.
Show a summary per file
| File | Description |
|---|---|
| services/web/server/tests/unit/with_dbs/01/storage/test_storage.py | Updated test cases include job_name in AsyncJobGet instance. |
| services/web/server/src/simcore_service_webserver/tasks/_rest.py | Changed task_name assignment from job.job_id to job.job_name. |
| services/storage/tests/unit/test_rpc_handlers_simcore_s3.py | Updated to spy on the new set_task_progress method. |
| services/storage/tests/unit/test_modules_celery.py | Updated task submission calls to use submit_task and adjusted dreamer_task signature. |
| services/storage/tests/unit/test_async_jobs.py | Updated AsyncJobGet creation with job_name. |
| services/storage/src/simcore_service_storage/modules/celery/worker.py | Renamed set_progress to set_task_progress. |
| services/storage/src/simcore_service_storage/modules/celery/models.py | Introduced TaskName type alias and extended Task model. |
| services/storage/src/simcore_service_storage/modules/celery/client.py | Changed send_task to submit_task and updated task metadata handling. |
| Other files (backends, RPC endpoints, REST endpoints, worker tasks, models-library) | Adjusted implementation to support the additional job_name field. |
Comments suppressed due to low confidence (1)
services/storage/tests/unit/test_modules_celery.py:92
- The function 'dreamer_task' now requires an extra parameter 'task_id', but callers using submit_task do not supply this parameter. Consider updating the task registration or reverting the function signature to ensure consistent invocation.
async def dreamer_task(task: AbortableTask, task_id: TaskID) -> list[int]:
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.
thx!
packages/models-library/src/models_library/api_schemas_rpc_async_jobs/async_jobs.py
Outdated
Show resolved
Hide resolved
services/storage/src/simcore_service_storage/modules/celery/models.py
Outdated
Show resolved
Hide resolved
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.
Thx!
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.
🚀
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.
Very nice! I really like the solution with the pipeline. Thanks a lot for the effort. Left some small suggestions.
services/storage/src/simcore_service_storage/modules/celery/backends/_redis.py
Show resolved
Hide resolved
services/storage/src/simcore_service_storage/modules/celery/backends/_redis.py
Show resolved
Hide resolved
services/storage/src/simcore_service_storage/modules/celery/backends/_redis.py
Outdated
Show resolved
Hide resolved
services/storage/src/simcore_service_storage/modules/celery/backends/_redis.py
Outdated
Show resolved
Hide resolved
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.
Thanks!
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.
some minor things
services/storage/src/simcore_service_storage/modules/celery/backends/_redis.py
Outdated
Show resolved
Hide resolved
services/storage/src/simcore_service_storage/modules/celery/backends/_redis.py
Outdated
Show resolved
Hide resolved
…hen-listing-tasks
…arloromeo/osparc-simcore into is7528/add-name-when-listing-tasks
|



What do these changes do?
This PR enhances the storage service by adding support for storing and retrieving the task (job) name when listing Celery tasks. Key changes include:
When tasks are listed, the
task_nameis included into the response.Related issue/s
task_namewhen listing Celery tasks #7528How to test
Dev-ops checklist