feat: Execute work item API

[priyadarshini-ni]

• This contribution adheres to CONTRIBUTING.md.

What does this Pull Request accomplish?

This PR includes client method for niworkitem/v1/{workitemid}/execute

Why 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

[rbell517]

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?

[darrenbiel]

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.

[ShriramS-Emerson]

@rbell517 So as to have the error as well as the execution result for certain status codes (as documented in the swagger page), I've added a new field response_data in the ApiException itself. Then in the respective function in the client, I've handled the exception and interpreted the response.

try:
    return self._execute_work_item(work_item_id=work_item_id, action=action)
except core.ApiException as e:
    if e.http_status_code in _EXECUTE_RESPONSE_STATUS_CODES and e.response_data:
         return models.ExecuteWorkItemResponse.model_validate(e.response_data)
    raise

Let me know if this approach looks fine.

[rbell517]

Sorry after Darren's message I had intended to come back with more detailed instructions but I slipped. What I was thinking when suggesting we override the base client was that for the execute route we would define our own response_handler hook on the execute route. I believe that will run prior to the base class response handler and allow us to extract the execute specific error model and let us raise our own exception type.

    @execute_error_response_handler
    @post(
        "workitems/{workItemId}/execute",
        args=[Path(name="workItemId"), Field("action")],
    )
    def execute_work_item(
        self, work_item_id: str, action: str
    ) -> models.ExecuteWorkItemResponse:
        ...
    
    @response_handler
    def execute_error_response_handler(response: Response):
        if 200 <= response.status_code < 300 or response.status_code > 499:
            # let the base class handle these
            return response
        
        # details...
        raise WorkItemExecuteApiException( ... )

    class WorkItemExecuteApiException(ApiException):
        ...
        
        @property
        def result(self) -> ExecutionResult | None:
            return self._result

This gives full control over the behavior and we stick to the existing semantics of errors raising exceptions, but with a new well typed exception.

[rbell517]

Sorry after Darren's message I had intended to come back with more detailed instructions but I slipped. What I was thinking when suggesting we override the base client was that for the execute route we would define our own response_handler hook on the execute route. I believe that will run prior to the base class response handler and allow us to extract the execute specific error model and let us raise our own exception type.

    @execute_error_response_handler
    @post(
        "workitems/{workItemId}/execute",
        args=[Path(name="workItemId"), Field("action")],
    )
    def execute_work_item(
        self, work_item_id: str, action: str
    ) -> models.ExecuteWorkItemResponse:
        ...
    
    @response_handler
    def execute_error_response_handler(response: Response):
        if 200 <= response.status_code < 300 or response.status_code > 499:
            # let the base class handle these
            return response
        
        # details...
        raise WorkItemExecuteApiException( ... )

    class WorkItemExecuteApiException(ApiException):
        ...
        
        @property
        def result(self) -> ExecutionResult | None:
            return self._result

This gives full control over the behavior and we stick to the existing semantics of errors raising exceptions, but with a new well typed exception.

[rbell517]

It strikes me as odd that we would expect to get this different error behavior for 403, 404, and 500, but not 400 or 401, etc.

When it comes to the implementation of the response handler, we should probably assume it could be from any error status code and determine if we need the execute specific model based on whether the response body has the fields we expect. That could be explicitly checking fields exist or a validate call on the Pydantic model in a try