Redesign base path file structure #373
Conversation
PhilippeMoussalli
changed the title
|
Thanks @PhilippeMoussalli! Is the manifest now also saved in the same directory as the data when running on Kubeflow? Or is it only written as a Kubeflow artifact? |
In both actually, It was done in #322 The kubeflow artifact one (which can't be modified) is needed to connect components through artifacts and the data one (in the base path) is for us |
src/fondant/executor.py
Outdated
| f"{manifest.pipeline_name}/{manifest.run_id}/" | ||
| f"{manifest.component_id}/manifest.json" |
There was a problem hiding this comment.
Then don't we still need the base_path here?
There was a problem hiding this comment.
Good catch! I confused it with a subset where the base_path is appended to the location
| @@ -212,9 +212,9 @@ def upload_manifest(self, manifest: Manifest, save_path: t.Union[str, Path]): | |||
|
|
|||
| if is_kubeflow_output: | |||
There was a problem hiding this comment.
I'm not the biggest fan of having kubeflow specific code in here. Opened #376 to remove this.
There was a problem hiding this comment.
I agree, though I don't see a clear-cut solution for this just yet
There was a problem hiding this comment.
will vertex have the same issue ?
There was a problem hiding this comment.
probably, ideally we have some control over the format and save path of the artifact.
Addresses #368
First review this #372