-
Notifications
You must be signed in to change notification settings - Fork 244
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
feat: API to list executions and filter by entity type #1103
base: main
Are you sure you want to change the base?
Conversation
… pagination, ordering
Quality Gate passedIssues Measures |
|
@@ -250,6 +250,7 @@ def get_required_setting( | |||
"prompt_studio.prompt_studio_document_manager_v2", | |||
"prompt_studio.prompt_studio_index_manager_v2", | |||
"tags", | |||
"execution", |
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.
Why do we need a new execution application instead of using our existing workflow_manager/workflow? Is there a specific reason for separating it from the existing setup?
Is this not part of workflow_execution?
"""Fetch the workflow name using workflow_id""" | ||
# TODO: Update after making Workflow a foreign key | ||
# return obj.workflow.workflow_name if obj.workflow_id else None | ||
if workflow := Workflow.objects.filter(id=obj.workflow_id).first(): |
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.
Can we use .get
instaed of filter and first?
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.
I think, It would be beteter if we add it in Workflow moodel as a property or method
return None | ||
|
||
# Check if pipeline_id exists in Pipeline model | ||
pipeline = Pipeline.objects.filter(id=obj.pipeline_id).first() |
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.
same feedback explaned for workflow
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.
May be Some helper function also avaialble in pipline or APIdeployment
queryset = WorkflowExecution.objects.all() | ||
|
||
# Filter based on execution entity | ||
if execution_entity == "API": |
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.
Can you use this as enum?
# Parse and apply date filters | ||
date_range_serializer = DateRangeSerializer(data=self.request.query_params) | ||
date_range_serializer.is_valid(raise_exception=True) |
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.
NIT: If we use this one as seriealizer, Add this as a base cass in the main serializer if possible. Another possibility , can we use DateTimeProcessor and filter class here?
Please remove migration files if not required |
What
created_at
API added
start_date
/end_date
/ordering
Why
How
executions
to provide this API using the existingWorkflowExecution
modelCan this PR break any existing features. If yes, please list possible items. If no, please explain why. (PS: Admins do not merge the PR without this section filled)
Database Migrations
Env Config
Relevant Docs
Related Issues or PRs
Dependencies Versions
Notes on Testing
Screenshots
Checklist
I have read and understood the Contribution Guidelines.