-
Notifications
You must be signed in to change notification settings - Fork 59
Update flyteplugins
#578
Update flyteplugins
#578
Conversation
Signed-off-by: Bernhard Stadlbauer <[email protected]>
1a6c9e9
to
ee92ceb
Compare
Signed-off-by: Bernhard Stadlbauer <[email protected]>
Signed-off-by: Bernhard Stadlbauer <[email protected]>
@hamersaw Not quite sure what's failing the docker build step here 🤔 I saw |
Seems to be an issue with go version in the propeller Dockerfile.
I'll get this fixed. |
Signed-off-by: Bernhard Stadlbauer <[email protected]>
if err != nil { | ||
return "", err | ||
return "", nil |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Is returning nil
here intended? Shouldn't this surface the error?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Not intentional 🤦 Thanks for spotting, fixed in 4a19d66
Signed-off-by: Bernhard Stadlbauer <[email protected]>
* Update `flyteplugins` Signed-off-by: Bernhard Stadlbauer <[email protected]> * Update `flytepropeller` Signed-off-by: Bernhard Stadlbauer <[email protected]> * Fix imports Signed-off-by: Bernhard Stadlbauer <[email protected]> * Bump flyteplugins to v1.1.7 Signed-off-by: Bernhard Stadlbauer <[email protected]> * Return `err` instead of `nil` Signed-off-by: Bernhard Stadlbauer <[email protected]> --------- Signed-off-by: Bernhard Stadlbauer <[email protected]> Co-authored-by: Dan Rammer <[email protected]>
TL;DR
Updates
flyteplugins
to the latest version.Removes all literal map hashing logic and replaces it with the one refactored into
flyteplugins
in flyteorg/flyteplugins#363Type
Are all requirements met?