Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Flytekit MultipartBlob/CSV should support single files as well (degenerate case) #128

Closed
kumare3 opened this issue Nov 26, 2019 · 2 comments
Labels
enhancement New feature or request

Comments

@kumare3
Copy link
Contributor

kumare3 commented Nov 26, 2019

Currently multipart CSV or Blob only support uploading local directories
https://github.com/lyft/flytekit/blob/master/flytekit/common/types/impl/blobs.py#L47-L53

If a director is not given it assumes the string is a remote path. This is a problem.
Ideally if the string is a file then we should upload the single file. Just adding a file check is not enough as upload() actually needs a directory

@kumare3 kumare3 added the bug Something isn't working label Nov 26, 2019
@matthewphsmith matthewphsmith added enhancement New feature or request and removed bug Something isn't working labels Dec 5, 2019
@matthewphsmith
Copy link
Contributor

This is by design so I wouldn't classify this as a bug. Happy to collect interest on it as an enhancement after considering potential repercussions.

@kumare3
Copy link
Contributor Author

kumare3 commented Jun 7, 2021

Resolving this, as in the new flytekit api, this has been resolved.

@kumare3 kumare3 closed this as completed Jun 7, 2021
eapolinario pushed a commit to eapolinario/flyte that referenced this issue Dec 6, 2022
eapolinario pushed a commit to eapolinario/flyte that referenced this issue Dec 6, 2022
eapolinario pushed a commit to eapolinario/flyte that referenced this issue Dec 20, 2022
* Add GetGormLogger

Signed-off-by: Katrina Rogan <[email protected]>

* Add GetGormLogger

Signed-off-by: Katrina Rogan <[email protected]>

* goimports

Signed-off-by: Katrina Rogan <[email protected]>

* no op

Signed-off-by: Katrina Rogan <[email protected]>

* try a thing

Signed-off-by: Katrina Rogan <[email protected]>
eapolinario pushed a commit to eapolinario/flyte that referenced this issue Dec 20, 2022
Signed-off-by: Flyte-Bot <[email protected]>

Co-authored-by: pmahindrakar-oss <[email protected]>
eapolinario pushed a commit to eapolinario/flyte that referenced this issue Aug 9, 2023
eapolinario pushed a commit to eapolinario/flyte that referenced this issue Aug 21, 2023
eapolinario pushed a commit to eapolinario/flyte that referenced this issue Aug 21, 2023
* Add GetGormLogger

Signed-off-by: Katrina Rogan <[email protected]>

* Add GetGormLogger

Signed-off-by: Katrina Rogan <[email protected]>

* goimports

Signed-off-by: Katrina Rogan <[email protected]>

* no op

Signed-off-by: Katrina Rogan <[email protected]>

* try a thing

Signed-off-by: Katrina Rogan <[email protected]>
eapolinario pushed a commit to eapolinario/flyte that referenced this issue Apr 30, 2024
Signed-off-by: Flyte-Bot <[email protected]>

Co-authored-by: pmahindrakar-oss <[email protected]>
pmahindrakar-oss pushed a commit that referenced this issue May 1, 2024
Just adding some unit tests. Unfortunately, because of where i wanted to add tests, I had to make the client an exported member of the struct instead.

Signed-off-by: Yee Hing Tong <[email protected]>
austin362667 pushed a commit to austin362667/flyte that referenced this issue May 7, 2024
Signed-off-by: Flyte-Bot <[email protected]>

Co-authored-by: pmahindrakar-oss <[email protected]>
robert-ulbrich-mercedes-benz pushed a commit to robert-ulbrich-mercedes-benz/flyte that referenced this issue Jul 2, 2024
Signed-off-by: Flyte-Bot <[email protected]>

Co-authored-by: pmahindrakar-oss <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
None yet
Development

No branches or pull requests

2 participants