-
Notifications
You must be signed in to change notification settings - Fork 27
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
[CDF-21657] Add status filter to workflow executions list #1793
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.
a test would be nice 👍 .... otherwise LGTM ✔️
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## master #1793 +/- ##
==========================================
- Coverage 92.93% 92.90% -0.04%
==========================================
Files 121 121
Lines 17691 17696 +5
==========================================
- Hits 16441 16440 -1
- Misses 1250 1256 +6
|
cognite/client/_api/workflows.py
Outdated
@@ -212,6 +212,7 @@ def list( | |||
workflow_version_ids: WorkflowVersionIdentifier | MutableSequence[WorkflowVersionIdentifier] | None = None, | |||
created_time_start: int | None = None, | |||
created_time_end: int | None = None, | |||
statuses: list[Literal["completed", "failed", "running", "terminated", "timed_out"]] | None = None, |
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 also allow a single status to be passed in, not just a list? It is a minor thing, but makes the users life a bit easier
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.
Any reason why this list is different than? https://github.com/cognitedata/cognite-sdk-python/blob/master/cognite/client/data_classes/workflows.py#L24
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 statuses there are the statuses for Tasks. The naming is a bit unfortunate. Would be a potential breaking change.
+ correctly differentiate between Workflow and Task statuses
@@ -32,6 +32,8 @@ | |||
"skipped", | |||
] | |||
|
|||
WorkflowStatus: TypeAlias = Literal["completed", "failed", "running", "terminated", "timed_out"] |
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.
"paused" has been removed from the possible statuses.
https://github.com/cognitedata/service-contracts/pull/2535
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.
Nice clarification between Task and Workflow status. In addition, to a user friendly input. Good work
Description
Allows filtering data workflow executions by status
Checklist:
If a new method has been added it should be referenced in cognite.rst in order to generate docs based on its docstring.