[Housekeeping] Admin endpoint doesn't bubble up errors

[pvditt]

Describe the issue

The GetExecutionData endpoint in admin does not bubble up errors when fetching inputs/outputs. Instead, the outputs are set to empty causing for confusion. (Unused URL blobs are returned in the response )

What if we do not do this?

Just code cleanliness - potentially some confusion on how errors are getting bubbled up to clients.

Related component(s)

flyteadmin

Are you sure this issue hasn't been raised already?

• Yes

Have you read the Code of Conduct?

• Yes

[dosubot]

🤖

Hello @pvditt! I'm Dosu, an AI bot here to help you with your Flyte issues. While we're waiting for a human maintainer to join us, I'm available to help troubleshoot bugs, answer questions, and guide you through contributing to Flyte. Nice to meet you!

The GetExecutionData function in the ExecutionManager does not bubble up errors when fetching inputs/outputs because it is designed to handle errors internally and return a response regardless of whether the inputs/outputs are successfully fetched or not.

In the provided code, if there is an error while fetching the inputs or outputs, the error is logged and the function continues to the next step. The function does not stop execution or bubble up the error. Instead, it returns a response with the inputs and outputs that were successfully fetched, and any inputs or outputs that could not be fetched are set to nil.

This design choice could be due to a variety of reasons. One possible reason is to ensure that the function always returns a response, even if some data could not be fetched. This can be useful in situations where partial data is better than no data. However, this can also lead to confusion if the user is not aware that some data could not be fetched.

Here is the relevant code:

inputs, inputURLBlob, err := util.GetInputs(ctx, m.urlData, m.config.ApplicationConfiguration().GetRemoteDataConfig(),
	m.storageClient, executionModel.InputsURI.String())
if err != nil {
	return nil, err
}
outputs, outputURLBlob, err := util.GetOutputs(ctx, m.urlData, m.config.ApplicationConfiguration().GetRemoteDataConfig(),
	m.storageClient, util.ToExecutionClosureInterface(execution.Closure))
if err != nil {
	return nil, err
}
response := &admin.WorkflowExecutionGetDataResponse{
	Inputs:      inputURLBlob,
	Outputs:     outputURLBlob,

In this code, if there is an error while fetching the inputs or outputs (util.GetInputs or util.GetOutputs), the error is returned and the function continues to the next step. The Inputs and Outputs in the response are set to the inputURLBlob and outputURLBlob respectively, which could be nil if there was an error while fetching them.

You can find this code in the flyteadmin/pkg/manager/impl/execution_manager.go file.

As for the GetNodeExecutionData function in the ExecutionManager within the flyteadmin component, it is used to retrieve the data associated with a specific node execution. This function takes in a NodeExecutionGetDataRequest which contains the details of the node execution for which the data is to be retrieved. It returns a NodeExecutionGetDataResponse which contains the data associated with the node execution.

In the provided context, the GetNodeExecutionData function is part of a mock NodeExecutionManager and is used for testing purposes. The actual implementation of the function would be set using the SetGetNodeExecutionDataFunc method. If no implementation is set, the function will return nil, nil.

You can find this code in the flyteadmin/pkg/manager/mocks/node_execution.go file.

I hope this clarifies your question. If you have any further questions, feel free to ask.

-Dosu

Sources

• flyteadmin/pkg/manager/impl/execution_manager.go
• flyteadmin/pkg/manager/mocks/node_execution.go

---

This response is meant to be useful and save you time. It is not meant to be a precise solution, but rather a starting point for your own research.

Help me be more useful! Please leave a 👍 if this is helpful and 👎 if it is irrelevant.

If you want to continue the conversation, start your reply with @dosu-bot.