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

Skyplane flytekit plugin #2860

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

Conversation

10sharmashivam
Copy link

@10sharmashivam 10sharmashivam commented Oct 24, 2024

Tracking issue

Reference Issue- Skyplane flytekit plugin

Why are the changes needed?

Skyplane plugin adds the capability of transferring data using Skyplane to Flyte. Skyplane allows for significantly faster and cheaper data transfers compared to traditional methods, enhancing the overall efficiency of data workflows.

What changes were proposed in this pull request?

•	Implementation of a new Flyte plugin to facilitate data transfers using Skyplane.
•	Definition of SkyplaneJob for configuring data transfer jobs.
•	Creation of SkyplaneFunctionTask to integrate with Flyte, including command construction for Skyplane operations.
•	Registration of the task plugin within Flyte’s plugin system.

How was this patch tested?

I have not been able to run a full end-to-end test as I currently do not have access to cloud storage locations. However, I validated the general structure of the plugin, and I’d appreciate any recommendations for accessible cloud storage locations for testing purposes.

Additional Comments

I’m submitting this PR as an initial implementation of the Skyplane plugin integration. At this stage, I’ve structured the plugin, created the SkyplaneJob and SkyplaneFunctionTask classes, and registered the plugin with Flyte.

And as mentioned in some old comments on parent issue for this PR, about launching machines and apache arrow based file system (I believe that’s what arrow FS means there, please let me know if I am correct), how can I go about it?

I’d greatly appreciate any feedback or guidance to ensure that I’m on the right track with this implementation, especially regarding the integration with Skyplane’s data transfer features. I’ll continue to refine the code and address any changes based on the team’s suggestions.

Looking forward to your insights!

Next Steps-

I plan to enhance error handling and logging in the upcoming iterations of this Skyplane plugin.
And likewise, this plugin will mature.

Check all the applicable boxes

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

Copy link

welcome bot commented Oct 24, 2024

Thank you for opening this pull request! 🙌

These tips will help get your PR across the finish line:

  • Most of the repos have a PR template; if not, fill it out to the best of your knowledge.
  • Sign off your commits (Reference: DCO Guide).

Copy link

codecov bot commented Oct 24, 2024

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 95.08%. Comparing base (3fc51af) to head (bc6c08f).
Report is 8 commits behind head on master.

Additional details and impacted files
@@             Coverage Diff             @@
##           master    #2860       +/-   ##
===========================================
+ Coverage   45.53%   95.08%   +49.55%     
===========================================
  Files         196       38      -158     
  Lines       20418     1527    -18891     
  Branches     2647        0     -2647     
===========================================
- Hits         9298     1452     -7846     
+ Misses      10658       75    -10583     
+ Partials      462        0      -462     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

@pingsutw pingsutw changed the title https://github.com/flyteorg/flyte/issues/3005 Skyplane flytekit plugin Oct 24, 2024
Signed-off-by: 10sharmashivam <[email protected]>
@10sharmashivam
Copy link
Author

Quick update: I’ve tried implementing the error handling and logging as planned. This should make debugging and future maintenance easier. Let me know if there’s anything specific you’d like me to adjust!

@davidmirror-ops
Copy link

@10sharmashivam thank you! as per testing, looks like AWS Free Tier includes 5GB of S3 standard storage, would you be able to try it?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
Status: Noteworthy PRs
Development

Successfully merging this pull request may close these issues.

2 participants