astronomer / astro-provider-ray

This provider contains operators, decorators and triggers to send a ray job from an airflow task
https://astronomer.github.io/astro-provider-ray/
Apache License 2.0
12 stars 2 forks source link

SubmitRayJob enhancements #50

Closed venkatajagannath closed 2 months ago

venkatajagannath commented 2 months ago

This PR contains the following changes -

Enhancements to the SubmitRayJob operator

Based on customer feedback, we learnt that it would be a much easier UX to spin up/down the cluster in the background of a task. The user would simply decorate their python function with @ray.task and the decorator would orchestrate the rest.

To enable this feature, we had to make changes to the code for SetupRayCluster and DeleteRayCluster operators. Making these changes helps us avoid duplication.

Add more more example DAGs

Earlier we had only 2 example dags. We now have 4. And we execute a different DAG for integration test.

Making the Decorator more robust

We made some changes to the decorator source code to make it more robust

Unit tests updated

Added unit tests where necessary and deleted where unnecessary. Updated where required.

Documentation improvements

Note: This PR does not contain any breaking changes.

codecov-commenter commented 2 months ago

Codecov Report

Attention: Patch coverage is 97.26776% with 5 lines in your changes missing coverage. Please review.

Project coverage is 97.80%. Comparing base (42a918d) to head (4d9c6b9). Report is 1 commits behind head on main.

Files with missing lines Patch % Lines
ray_provider/hooks/ray.py 96.36% 4 Missing :warning:
ray_provider/decorators/ray.py 96.00% 1 Missing :warning:
Additional details and impacted files ```diff @@ Coverage Diff @@ ## main #50 +/- ## ========================================== + Coverage 97.12% 97.80% +0.68% ========================================== Files 5 5 Lines 521 546 +25 ========================================== + Hits 506 534 +28 + Misses 15 12 -3 ```

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

venkatajagannath commented 2 months ago

@venkatajagannath thanks for advancing on this and addressing customer's feedback!

Could you highlight in the PR description if/ what are the breaking changes of this PR?

Also, assuming there are breaking changes, would it make sense to release an alpha with this change (e.g. 0.2.0a1), so we can iterate with the customers on the desired interface/feature quickly? We can have as many alpha releases as we want without impacting other people

@tatiana There are no breaking changes in the release. It just simplifies cluster spin up and down. So in a way its actually an enhancement to an existing operator. Sure, we can do an alpha release.

Will update the PR description.

pankajastro commented 2 months ago

It just simplifies cluster spin up and down. So in a way its actually an enhancement to an existing operator. Sure, we can do an alpha release.

If this is urgent you can also cut alpha from this branch