-
Notifications
You must be signed in to change notification settings - Fork 11
Handle processing_service_name parameters in requests from workers #1117
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?
Changes from all commits
b60eab0
644927f
218f7aa
02aa6fa
90d729f
335229d
117057f
5b53380
90da389
8618d3c
bd1be5f
ba74747
595f4c9
0d8c649
751573e
7a28477
2554950
9ae607c
3f4729b
cae6ff3
9cb886b
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -15,7 +15,7 @@ | |
|
|
||
| from ami.base.permissions import ObjectPermission | ||
| from ami.base.views import ProjectMixin | ||
| from ami.jobs.schemas import batch_param, ids_only_param, incomplete_only_param | ||
| from ami.jobs.schemas import batch_param, ids_only_param, incomplete_only_param, processing_service_name_param | ||
| from ami.jobs.tasks import process_nats_pipeline_result | ||
| from ami.main.api.schemas import project_id_doc_param | ||
| from ami.main.api.views import DefaultViewSet | ||
|
|
@@ -204,13 +204,16 @@ def get_queryset(self) -> QuerySet: | |
| project_id_doc_param, | ||
| ids_only_param, | ||
| incomplete_only_param, | ||
| processing_service_name_param, | ||
| ] | ||
| ) | ||
| def list(self, request, *args, **kwargs): | ||
| _ = _log_processing_service_name(request, "list requested", logger) | ||
carlosgjs marked this conversation as resolved.
Show resolved
Hide resolved
|
||
|
|
||
| return super().list(request, *args, **kwargs) | ||
|
|
||
| @extend_schema( | ||
| parameters=[batch_param], | ||
| parameters=[batch_param, processing_service_name_param], | ||
| responses={200: dict}, | ||
| ) | ||
| @action(detail=True, methods=["get"], name="tasks") | ||
|
|
@@ -229,6 +232,7 @@ def tasks(self, request, pk=None): | |
| except Exception as e: | ||
| raise ValidationError({"batch": str(e)}) from e | ||
|
|
||
| _ = _log_processing_service_name(request, f"tasks ({batch}) requested for job {job.pk}", job.logger) | ||
| # Only async_api jobs have tasks fetchable from NATS | ||
| if job.dispatch_mode != JobDispatchMode.ASYNC_API: | ||
| raise ValidationError("Only async_api jobs have fetchable tasks") | ||
|
|
@@ -254,6 +258,9 @@ async def get_tasks(): | |
|
|
||
| return Response({"tasks": tasks}) | ||
|
|
||
| @extend_schema( | ||
| parameters=[processing_service_name_param], | ||
| ) | ||
| @action(detail=True, methods=["post"], name="result") | ||
| def result(self, request, pk=None): | ||
| """ | ||
|
|
@@ -266,6 +273,8 @@ def result(self, request, pk=None): | |
|
|
||
| job = self.get_object() | ||
|
|
||
| _ = _log_processing_service_name(request, f"result received for job {job.pk}", job.logger) | ||
|
|
||
| # Validate request data is a list | ||
| if isinstance(request.data, list): | ||
| results = request.data | ||
|
|
@@ -325,3 +334,24 @@ def result(self, request, pk=None): | |
| }, | ||
| status=500, | ||
| ) | ||
|
|
||
|
|
||
| def _log_processing_service_name(request, context: str, logger: logging.Logger) -> str | None: | ||
| """ | ||
| Log the processing_service_name from query parameters. | ||
|
|
||
| Args: | ||
| request: The HTTP request object | ||
| context: A string describing the operation (e.g., "tasks requested", "result received") | ||
| logger: A logging.Logger instance to use for logging | ||
carlosgjs marked this conversation as resolved.
Show resolved
Hide resolved
|
||
| Returns: | ||
| The processing_service_name if provided, otherwise None | ||
| """ | ||
| processing_service_name = request.query_params.get("processing_service_name", None) | ||
|
|
||
| if processing_service_name: | ||
| logger.info(f"Jobs {context} by processing service: {processing_service_name}") | ||
carlosgjs marked this conversation as resolved.
Show resolved
Hide resolved
|
||
| else: | ||
| logger.debug(f"Jobs {context} without processing service name") | ||
|
Comment on lines
+350
to
+355
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Sanitize user-supplied
Additionally, f-strings force eager string evaluation regardless of the effective log level; use 🛡️ Proposed fix – sanitize input and use lazy log formatting- if processing_service_name:
- logger.info(f"Jobs {context} by processing service: {processing_service_name}")
- else:
- logger.debug(f"Jobs {context} without processing service name")
+ if processing_service_name:
+ # Sanitize to prevent log injection via newlines / carriage returns
+ sanitized_name = processing_service_name.replace("\n", "\\n").replace("\r", "\\r")
+ logger.info("Jobs %s by processing service: %s", context, sanitized_name)
+ else:
+ logger.debug("Jobs %s without processing service name", context)🤖 Prompt for AI Agents |
||
|
|
||
| return processing_service_name | ||
Uh oh!
There was an error while loading. Please reload this page.