Skip to content
This repository has been archived by the owner on Oct 9, 2023. It is now read-only.

Add support upload collections #54

Open
wants to merge 2 commits into
base: master
Choose a base branch
from

Conversation

pingsutw
Copy link
Member

@pingsutw pingsutw commented Mar 9, 2023

TL;DR

Add support upload collection (a list).
When using raw container in the map task, flytepropeller expect the subtask upload a list.

Type

  • Bug Fix
  • Feature
  • Plugin

Are all requirements met?

  • Code completed
  • Smoke tested
  • Unit tests added
  • Code documentation added
  • Any pending items have an associated Issue

Complete description

^^^

Tracking Issue

https://flyte-org.slack.com/archives/CP2HDHKE1/p1678230956906899

Follow-up issue

NA

@pingsutw pingsutw requested review from EngHabu and kumare3 as code owners March 9, 2023 21:51
@codecov
Copy link

codecov bot commented Mar 9, 2023

Codecov Report

Merging #54 (4328d82) into master (b1ba2f7) will increase coverage by 3.23%.
The diff coverage is 65.51%.

❗ Current head 4328d82 differs from pull request most recent head 92d5fb9. Consider uploading reports for the commit 92d5fb9 to get more accurate results

@@            Coverage Diff             @@
##           master      #54      +/-   ##
==========================================
+ Coverage   17.79%   21.02%   +3.23%     
==========================================
  Files           3        3              
  Lines         399      428      +29     
==========================================
+ Hits           71       90      +19     
- Misses        315      320       +5     
- Partials       13       18       +5     
Flag Coverage Δ
unittests ?

Flags with carried forward coverage won't be shown. Click here to find out more.

Impacted Files Coverage Δ
data/upload.go 39.04% <65.51%> (+6.56%) ⬆️

Help us with your feedback. Take ten seconds to tell us how you rate us. Have a feature suggestion? Share it here.

Signed-off-by: Kevin Su <[email protected]>
Copy link
Contributor

@wild-endeavor wild-endeavor left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

+1 but a bit confused... why do we need to do this? I thought for map tasks propeller is the one that assembles the list. and the individual tasks still upload single scalar items.

@pingsutw
Copy link
Member Author

pingsutw commented Mar 20, 2023

@pingsutw
Copy link
Member Author

because flytekit will convert output to a list literal. https://github.com/flyteorg/flytekit/blob/93995e29dcd37317e91ebcfb3019a4ffed0e82de/flytekit/core/map_task.py#LL56C47-L56C47

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants