Return full execution data on every request if under max specified size #109
Conversation
katrogan
requested review from
anandswaminathan,
EngHabu,
kumare3,
matthewphsmith and
wild-endeavor
as code owners
Codecov Report
@@ Coverage Diff @@
## master #109 +/- ##
==========================================
+ Coverage 62.40% 62.55% +0.15%
==========================================
Files 104 104
Lines 7652 7716 +64
==========================================
+ Hits 4775 4827 +52
- Misses 2314 2321 +7
- Partials 563 568 +5
Flags with carried forward coverage won't be shown. Click here to find out more.
Continue to review full report at Codecov.
|
| @@ -1019,10 +1021,32 @@ func (m *ExecutionManager) GetExecutionData( | |||
| if err != nil { | |||
| return nil, err | |||
| } | |||
| return &admin.WorkflowExecutionGetDataResponse{ | |||
| response := &admin.WorkflowExecutionGetDataResponse{ | |||
There was a problem hiding this comment.
@katrogan one reason to do this is to avoid having to sign the URL, so can we just do a HEAD, check the size and if that is ok then do a protobuf get using flytestdlib.storage?
There was a problem hiding this comment.
or maybe i am misunderstanding, do we actually store the size of the payload in our database?
There was a problem hiding this comment.
until the UI transitions to using the new field, we should return both for backwards compatibility
but good point on using flytestdlib, will update
There was a problem hiding this comment.
no, we don't store the size
There was a problem hiding this comment.
oh wait, and the storage client is using flytestdlib already
TL;DR
This change returns fully populated input & output data on GetExecutionData requests in addition to signed URLs.
Not to be merged until flyteorg/flyteidl#73 is checked in & released
Type
Are all requirements met?
Complete description
This is useful to return inputs & outputs in sandbox mode when the signed url is host-specific. Also fixes the leaky abstraction presented by requiring a caller to concern themselves with where the data stored and having to parse the proto on their own.
Tracking Issue
flyteorg/flyte#419
Follow-up issue
NA
OR
https://github.com/lyft/flyte/issues/