Conversation
nisystemlink/clients/work_item/models/_execute_work_item_request.py
Outdated
Show resolved
Hide resolved
nisystemlink/clients/work_item/models/_execute_work_item_response.py
Outdated
Show resolved
Hide resolved
nisystemlink/clients/work_item/models/_execute_work_item_response.py
Outdated
Show resolved
Hide resolved
| """Error information if the action failed.""" | ||
|
|
||
| result: ExecutionResult | None = None | ||
| """Result of the action execution.""" |
There was a problem hiding this comment.
I want to know if we can clean up this response model or if its useful to have these extra layers. Best I can tell the error will only be set when the response is an error status code, which we handle in the base client to convert into an ApiError already, so this part of the model will never be used. That leaves just the result, so can we elevate that and return it directly from the client instead?
Conversely, if we do need to return both the error and the execution result together in the error case then we need to override the base client's status code handler so it doesn't throw so we can have the behavior we'd want.
@darrenbiel or @kaviarasu-ni can you comment on the way we intend the API to behave?
There was a problem hiding this comment.
The service may return both an error and job IDs in a partial failure case. It will cancel the jobs it successfully queued and return their IDs along with the error. The error is duplicated at the base response and in the Result.Error.
I think we would need to override the base client as you suggested.
| execution_id: str | None = None | ||
| """The notebook execution ID. Only populated when type is NOTEBOOK.""" | ||
|
|
||
| job_ids: List[str] | None = None | ||
| """The list of job IDs. Only populated when type is JOB.""" |
There was a problem hiding this comment.
If these are only used for their corresponding job types then we should make different model objects for these rather than relying on comments.
Ideally this would be defined as a discriminated union.
class ExecutionResultBase(JsonModel):
"""Result of executing a work item action."""
type: Literal["NONE", "MANUAL", "NOTEBOOK", "JOB", "SCHEDULE", "UNSCHEDULE"]
"""Type of execution."""
class JobExecutionResult(ExecutionResultBase):
type: Literal["JOB"]
"""Type of execution."""
job_ids: List[str] | None = None
"""The list of job IDs."""
class NotebookExecutionResult(ExecutionResultBase):
type: Literal["NOTEBOOK"]
"""Type of execution."""
execution_id: str | None = None
"""The notebook execution ID."""
...
ExecutionResult = Annotated[
Union[
NoneExecutionResult,
ManualExecutionResult,
NotebookExecutionResult,
JobExecutionResult,
ScheduleExecutionResult,
UnscheduleExecutionResult
],
Field(discriminator='type')
]
"""Result of executing a work item action."""
class ExecuteWorkItemResponse(JsonModel):
"""Response for executing a work item action."""
result: ExecutionResult
"""Result of the action execution."""
What does this Pull Request accomplish?
This PR includes client method for
niworkitem/v1/{workitemid}/executeWhy should this Pull Request be merged?
This API endpoint can be used to execute the workflow attached to a work item
What testing has been done?
Automated testing has been included