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

Runtime reorg #230

Merged
merged 6 commits into from
Jun 5, 2024
Merged

Runtime reorg #230

merged 6 commits into from
Jun 5, 2024

Conversation

daw3rd
Copy link
Member

@daw3rd daw3rd commented Jun 4, 2024

Why are these changes needed?

Continued work on splitting transforms. For now this includes filter and some fixes for noop.
Extends the test-image target to not just run pytest, but to try and execute the transform using --help.
Fixes non-python transform runtimes by properly installing the adjacent python transform into the venv

Related issue number (if any).

@daw3rd daw3rd requested review from roytman, cmadam and blublinsky June 4, 2024 21:20
@daw3rd
Copy link
Member Author

daw3rd commented Jun 4, 2024

@roytman approval or not I will wait on your ok before any merging of this. KFP v2 has priority so if this somehow conflicts (although it may also fix some issues), this can wait until kfp is merged.

Of course, you should feel free to merge in my absence.

@roytman
Copy link
Member

roytman commented Jun 5, 2024

@roytman approval or not I will wait on your ok before any merging of this. KFP v2 has priority so if this somehow conflicts (although it may also fix some issues), this can wait until kfp is merged.

Of course, you should feel free to merge in my absence.

@roytman approval or not I will wait on your ok before any merging of this. KFP v2 has priority so if this somehow conflicts (although it may also fix some issues), this can wait until kfp is merged.

Of course, you should feel free to merge in my absence.

@daw3rd thank you very much.

Copy link
Member

@roytman roytman left a comment

Choose a reason for hiding this comment

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

LGTM, I'm OK to merge it

Copy link
Collaborator

@blublinsky blublinsky left a comment

Choose a reason for hiding this comment

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

I am approving it, but this is unusable

from data_processing_ray.runtime.ray import RayTransformLauncher
from data_processing_ray.runtime.ray.runtime_configuration import (
RayTransformRuntimeConfiguration,
)
from filter_transform import FilterTransformConfiguration
Copy link
Collaborator

Choose a reason for hiding this comment

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

Oh, its a monster, that is based on a clever make files, but requires a lot of pycharm knowledge and very hard to understand and reason

@revit13 revit13 merged commit 676625a into dev Jun 5, 2024
14 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants