-
Notifications
You must be signed in to change notification settings - Fork 25
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
Split component and pipeline SDKs #587
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.
Thanks Robbe!
@@ -27,7 +28,7 @@ def build_sidebar(base_path: str) -> Tuple[Manifest, str, Dict[str, str]]: | |||
Tuple[Optional[str], Optional[str], Optional[Dict]: Tuple with manifest path, | |||
subset name and fields | |||
""" | |||
fs = get_filesystem(base_path) | |||
fs, _ = fsspec.core.url_to_fs(base_path) |
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 find
) | ||
from fondant.pipeline import Pipeline | ||
from fondant.runner import DockerRunner, KubeflowRunner, VertexRunner | ||
if t.TYPE_CHECKING: |
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.
what does this do?
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.
It only imports the guarded imports during type checking, not at runtime. See docs.
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! Going to be clean
Fixes #554
This PR splits the component and pipeline SDKs. Some things I want to add in follow-up PRs: