flyteorg / flyte

Scalable and flexible workflow orchestration platform that seamlessly unifies data, ML and analytics stacks.
https://flyte.org
Apache License 2.0
5.79k stars 660 forks source link

[Core feature] Deprecate CreateDownloadLocation API to allow for a more controllable CreateDownloadLink API. #3034

Closed EngHabu closed 1 year ago

EngHabu commented 2 years ago

Motivation: Why do you think this is important?

There are a few issues with the current API:

Goal: What should the final outcome look like, ideally?

The request should probably change from:

// CreateDownloadLocationRequest specified request for the CreateDownloadLocation API.
message CreateDownloadLocationRequest {
  // NativeUrl specifies the url in the format of the configured storage provider (e.g. s3://my-bucket/randomstring/suffix.tar)
  string native_url = 1;
  // ExpiresIn defines a requested expiration duration for the generated url. The request will be rejected if this
  // exceeds the platform allowed max.
  // +optional. The default value comes from a global config.
  google.protobuf.Duration expires_in = 2;
}

to something like:

message CreateDownloadLinkRequest {
  oneof source {
    // WorkflowId is the unique identifier for the workflow execution.
    core.WorkflowExecutionIdentifier execution_id = 1;

    // NodeId is the unique identifier for the node execution. For a task node, this will retrieve the output of the
    // most recent attempt of the task.
    core.NodeExecutionIdentifier node_id = 2;

    // TaskId is the unique task execution identifier that points to a specific attempt of the task.
    core.TaskExecutionIdentifier task_id = 3;
  }

  ArtifactType artifact_type = 4;

  // ExpiresIn defines a requested expiration duration for the generated url. The request will be rejected if this
  // exceeds the platform allowed max.
  // +optional. The default value comes from a global config.
  google.protobuf.Duration expires_in = 5;
}

Describe alternatives you've considered

Admin parsing the request URL and mapping to an execution: puts a lot of assumptions on the schema of the URL.

Propose: Link/Inline OR Additional context

No response

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

Have you read the Code of Conduct?

eapolinario commented 1 year ago

@EngHabu , can we close this? Is this open to track the work to enable this in cloud?

cosmicBboy commented 1 year ago

@EngHabu ping ^^

EngHabu commented 1 year ago

Yes! This is all done