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

Extract kubeflow logic from ComponentSpec to KubeflowComponent #36

Merged
merged 2 commits into from
Apr 25, 2023

Conversation

RobbeSneyders
Copy link
Member

This PR extracts all the kubeflow logic from the ComponentSpec to the KubeflowComponent so the ComponentSpec is a minimal representation of the fondant component specification. I also brought the classes a bit more in line with the Manifest class.

This is preparation for the work to deduct the output manifest from the input manifest and component spec only.

Copy link
Contributor

@PhilippeMoussalli PhilippeMoussalli left a comment

Choose a reason for hiding this comment

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

Really nice refactoring 👍

Thanks!

cls, fondant_component: FondantComponentSpec
) -> "KubeflowComponentSpec":
"""Create a Kubeflow component spec from a Fondant component spec."""
specification = {
Copy link
Contributor

Choose a reason for hiding this comment

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

Nice :) more explicit than before

fondant/component_spec.py Outdated Show resolved Hide resolved

return dumped_args

def to_file(self, path: t.Union[str, Path]) -> None:
Copy link
Contributor

Choose a reason for hiding this comment

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

I noticed that when compiling the kfp pipeline you can pass in directly a json string instead of the yaml file. So might be worth switching over to that approach. But this should be handled in separate PR related when automating the pipeline definition and compilation

Copy link
Member Author

Choose a reason for hiding this comment

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

Ok, the json is available under _spec already, so should be easy to expose.

Copy link
Contributor

@NielsRogge NielsRogge left a comment

Choose a reason for hiding this comment

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

Thanks for improving!

@RobbeSneyders RobbeSneyders merged commit 1f562f9 into main Apr 25, 2023
@RobbeSneyders RobbeSneyders deleted the feature/component-spec branch May 15, 2023 16:29
Hakimovich99 pushed a commit that referenced this pull request Oct 16, 2023
This PR extracts all the kubeflow logic from the `ComponentSpec` to the
`KubeflowComponent` so the `ComponentSpec` is a minimal representation
of the fondant component specification. I also brought the classes a bit
more in line with the `Manifest` class.

This is preparation for the work to deduct the output manifest from the
input manifest and component spec only.
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.

3 participants