flytekit icon indicating copy to clipboard operation
flytekit copied to clipboard

Add support for remote.fetch much like `pyflyte fetch`

Open katrogan opened this issue 1 year ago • 3 comments

Why are the changes needed?

I think it's nice to have parity here. We could also choose to extend this in the future to support not just the flyte:// URI protocol but to actually support s3/gcs/etc paths too.

What changes were proposed in this pull request?

See PR title.

How was this patch tested?

I ran the following workflow in sandbox

from flytekit import task, workflow, Resources


from flytekit.types.file import FlyteFile

@task
def create_file() -> FlyteFile:
    with open("data.txt", "w") as f:
        f.write("Hello flyte world")
    return FlyteFile(path="data.txt")


@workflow
def create_small_file_wf() -> FlyteFile:
    f = create_file()
    return f

and then, with my local flytekit installation

from flytekit import Config, FlyteRemote

if __name__ == "__main__":
    remote = FlyteRemote(config=Config.for_sandbox())
    remote.fetch(uri="flyte://v1/flytesnacks/development/a96jjgcjssdmw5kksznp/n0/o", download_to="/Users/katrina/scratch/remotefetch")

And verified that data was fetched

$ ls /Users/katrina/scratch/remotefetch
o0
$ ls /Users/katrina/scratch/remotefetch/o0
data.txt
$ cat /Users/katrina/scratch/remotefetch/o0/data.txt 
Hello flyte world%  

Setup process

Screenshots

Check all the applicable boxes

  • [ ] I updated the documentation accordingly.
  • [ ] All new and existing tests passed.
  • [ ] All commits are signed-off.

Related PRs

Docs link

katrogan avatar Oct 31 '24 14:10 katrogan

cc @thomasjpfan

katrogan avatar Nov 05 '24 12:11 katrogan

Codecov Report

Attention: Patch coverage is 33.33333% with 2 lines in your changes missing coverage. Please review.

Project coverage is 72.41%. Comparing base (8feb3b8) to head (070cb57). Report is 2 commits behind head on master.

Files with missing lines Patch % Lines
flytekit/remote/remote.py 33.33% 2 Missing :warning:

:exclamation: There is a different number of reports uploaded between BASE (8feb3b8) and HEAD (070cb57). Click for more details.

HEAD has 2 uploads less than BASE
Flag BASE (8feb3b8) HEAD (070cb57)
3 1
Additional details and impacted files
@@             Coverage Diff             @@
##           master    #2886       +/-   ##
===========================================
- Coverage   94.25%   72.41%   -21.85%     
===========================================
  Files          18      199      +181     
  Lines        1358    20856    +19498     
  Branches        0     2684     +2684     
===========================================
+ Hits         1280    15103    +13823     
- Misses         78     5024     +4946     
- Partials        0      729      +729     

:umbrella: View full report in Codecov by Sentry.
:loudspeaker: Have feedback on the report? Share it here.

codecov[bot] avatar Nov 07 '24 14:11 codecov[bot]

can we meditate and percolate on this a bit more?

wild-endeavor avatar Nov 15 '24 18:11 wild-endeavor