Implement CreateRun and UploadInputs APIs#7152
Conversation
Signed-off-by: Katrina Rogan <katroganGH@gmail.com>
Signed-off-by: Katrina Rogan <katroganGH@gmail.com>
Signed-off-by: Katrina Rogan <katroganGH@gmail.com>
Signed-off-by: Katrina Rogan <katroganGH@gmail.com>
Signed-off-by: Katrina Rogan <katroganGH@gmail.com>
Signed-off-by: Katrina Rogan <katroganGH@gmail.com>
Signed-off-by: Katrina Rogan <katroganGH@gmail.com>
runs/service/run_service.go
Outdated
| inputs = request.GetInputs() | ||
| } | ||
|
|
||
| inputs = fillDefaultInputs(inputs, taskSpec.GetDefaultInputs()) |
There was a problem hiding this comment.
not sure how necessary this is for all cases, it's not ideal we read the offloaded inputs and then reupload them again below
There was a problem hiding this comment.
We use the inputs to generate the cache key at line 216. is it possible that dataproxy generates the same hash here, so we don't need to read the offload inputs?
There was a problem hiding this comment.
or I think we can only read the inputs only when cache is enabled at least.
There was a problem hiding this comment.
look at the sdk code, i think it's possible. we can pass a task template to UploadInputRequest if we want
There was a problem hiding this comment.
task template is in the task spec. https://github.com/flyteorg/flyte-sdk/blob/aad83a1bb9aa4690fd09c65858b68417208ff697/src/flyte/_run.py#L401-L407
There was a problem hiding this comment.
Never mind—ignore me. I think we just need to update generateCacheKeyForTask to make it the same as the closed-source version
There was a problem hiding this comment.
generateCacheKeyForTask(taskTemplate, precompute-hash)
Signed-off-by: Katrina Rogan <katroganGH@gmail.com>
runs/service/run_service.go
Outdated
| inputs = request.GetInputs() | ||
| } | ||
|
|
||
| inputs = fillDefaultInputs(inputs, taskSpec.GetDefaultInputs()) |
There was a problem hiding this comment.
We use the inputs to generate the cache key at line 216. is it possible that dataproxy generates the same hash here, so we don't need to read the offload inputs?
runs/service/run_service.go
Outdated
| inputs = request.GetInputs() | ||
| } | ||
|
|
||
| inputs = fillDefaultInputs(inputs, taskSpec.GetDefaultInputs()) |
There was a problem hiding this comment.
or I think we can only read the inputs only when cache is enabled at least.
…ate-createrun-uploadinputs-apis
Signed-off-by: Katrina Rogan <katroganGH@gmail.com>
… into katrina/eng26-346-implement-separate-createrun-uploadinputs-apis-in-oss
Signed-off-by: Katrina Rogan <katroganGH@gmail.com>
… into katrina/eng26-346-implement-separate-createrun-uploadinputs-apis-in-oss
Signed-off-by: Katrina Rogan <katroganGH@gmail.com>
katrogan
deleted the
katrina/eng26-346-implement-separate-createrun-uploadinputs-apis-in-oss
branch
Signed-off-by: Kevin Su <pingsutw@apache.org>
2 participants
Why are the changes needed?
Implements #7139 in OSS
What changes were proposed in this pull request?
Decouples uploading inputs from calling CreateRun and switches to support CreateRun with inputs passed by URI
How was this patch tested?
Tested locally by running devbox. The UI doesn't seem to like orgs even though they're required by the CRD so I verified the hello example locally like so
Labels
Please add one or more of the following labels to categorize your PR:
This is important to improve the readability of release notes.
Setup process
Screenshots
Check all the applicable boxes
Related PRs
Docs link
main