-
Notifications
You must be signed in to change notification settings - Fork 1.1k
feature: except / only fields for task and tasks endpoints #1436
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: master
Are you sure you want to change the base?
Conversation
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 adds field filtering functionality to task and tasks API endpoints through new except_fields and only_fields query parameters. This allows API consumers to selectively include or exclude specific fields from the response, which is useful for reducing payload size when certain fields (like large children lists) are not needed.
- Implements
filter_dictutility function to handle field filtering logic - Updates task and tasks endpoints to accept and process field filtering parameters
- Adds comprehensive test coverage for the new filtering functionality
Reviewed Changes
Copilot reviewed 3 out of 3 changed files in this pull request and generated 3 comments.
| File | Description |
|---|---|
| flower/utils/tasks.py | Adds filter_dict function and updates as_dict to support field filtering |
| flower/api/tasks.py | Updates task and tasks endpoints to accept filtering parameters and apply them |
| tests/unit/api/test_tasks.py | Adds test coverage for field filtering functionality and updates existing tests |
| response = tasks.as_dict(task, only_fields=only_fields, except_fields=except_fields) | ||
| if task.worker is not None and 'worker' in response: | ||
| response['worker'] = task.worker.hostname | ||
|
|
Copilot
AI
Aug 12, 2025
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.
The condition checks if 'worker' exists in the filtered response, but the worker hostname assignment happens after filtering. If 'worker' is excluded by filtering, this condition will prevent the assignment, but if 'worker' is included, the assignment will overwrite the filtered value. Consider applying the worker hostname transformation before filtering.
| response = tasks.as_dict(task, only_fields=only_fields, except_fields=except_fields) | |
| if task.worker is not None and 'worker' in response: | |
| response['worker'] = task.worker.hostname | |
| # Ensure the worker field is set to the hostname before filtering | |
| if task.worker is not None: | |
| task.worker = task.worker.hostname | |
| response = tasks.as_dict(task, only_fields=only_fields, except_fields=except_fields) |
| task = tasks.as_dict(task, only_fields=only_fields, except_fields=except_fields) | ||
| worker = task.pop('worker', None) | ||
| if worker is not None: | ||
| task['worker'] = worker.hostname |
Copilot
AI
Aug 12, 2025
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.
The worker hostname transformation on lines 530-532 happens after filtering, which means if 'worker' is included in only_fields or excluded from except_fields, the transformation may not work as expected. The worker field should be transformed before applying field filtering.
| task = tasks.as_dict(task, only_fields=only_fields, except_fields=except_fields) | |
| worker = task.pop('worker', None) | |
| if worker is not None: | |
| task['worker'] = worker.hostname | |
| if hasattr(task, 'worker') and task.worker is not None: | |
| task.worker = task.worker.hostname | |
| task = tasks.as_dict(task, only_fields=only_fields, except_fields=except_fields) |
| dict: The filtered dictionary | ||
| """ |
Copilot
AI
Aug 12, 2025
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.
The function doesn't validate mutual exclusivity of only_fields and except_fields parameters. If both are provided, only_fields takes precedence silently, which could lead to unexpected behavior. Consider adding validation to reject requests with both parameters or documenting this precedence clearly.
| dict: The filtered dictionary | |
| """ | |
| dict: The filtered dictionary | |
| Raises: | |
| ValueError: If both only_fields and except_fields are provided. | |
| """ | |
| if only_fields and except_fields: | |
| raise ValueError("Cannot specify both only_fields and except_fields. Please provide only one.") |
Adds
except_fieldsandonly_fieldsparams that take in field names as they appear intaskandtasksendpoints.My case was that I wanted to exclude huge
childrenkey list.