-
Notifications
You must be signed in to change notification settings - Fork 26
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
Conversation
There was a problem hiding this 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 = { |
There was a problem hiding this comment.
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
|
||
return dumped_args | ||
|
||
def to_file(self, path: t.Union[str, Path]) -> None: |
There was a problem hiding this comment.
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
There was a problem hiding this comment.
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.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks for improving!
9610aae
to
dff0568
Compare
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.
This PR extracts all the kubeflow logic from the
ComponentSpec
to theKubeflowComponent
so theComponentSpec
is a minimal representation of the fondant component specification. I also brought the classes a bit more in line with theManifest
class.This is preparation for the work to deduct the output manifest from the input manifest and component spec only.