Skip to content

Commit

Permalink
Mount absolute path of working dir to local runner (#931)
Browse files Browse the repository at this point in the history
As part of the new interface and manifest changes this bug appeared. We
are not able to run the new fondant version in jupyter notebooks cause
the working directory isn't resolved correctly.

Mounting the complete path instead of the stem solves the issue.
  • Loading branch information
mrchtr committed Apr 17, 2024
1 parent bee4b3b commit c8aa2b2
Show file tree
Hide file tree
Showing 2 changed files with 6 additions and 7 deletions.
8 changes: 4 additions & 4 deletions src/fondant/dataset/compiler.py
Original file line number Diff line number Diff line change
Expand Up @@ -180,10 +180,10 @@ def resolve_local_base_path(base_path: Path) -> Path:
volume = DockerVolume(
type="bind",
source=str(p_base_path),
target=f"/{p_base_path.stem}",
target=str(p_base_path),
)
path = f"/{p_base_path.stem}"
return path, volume

return str(p_base_path), volume

def _generate_spec(
self,
Expand Down Expand Up @@ -246,7 +246,7 @@ def _generate_spec(
command.extend(
[
"--working_directory",
working_directory,
path,
],
)

Expand Down
5 changes: 2 additions & 3 deletions tests/pipeline/test_compiler.py
Original file line number Diff line number Diff line change
Expand Up @@ -213,7 +213,6 @@ def test_docker_local_path(setup_pipeline, tmp_path_factory):
with tmp_path_factory.mktemp("temp") as fn:
# this is the directory mounted in the container
_, _, dataset, cache_dict = setup_pipeline
work_dir_stem = f"/{fn.stem}"
working_directory = str(fn)
compiler = DockerCompiler()
output_path = str(fn / "docker-compose.yml")
Expand All @@ -234,14 +233,14 @@ def test_docker_local_path(setup_pipeline, tmp_path_factory):
assert component_configs.volumes == [
{
"source": str(fn),
"target": work_dir_stem,
"target": str(fn),
"type": "bind",
},
]
cleaned_pipeline_name = dataset.name.replace("_", "")
# check if commands are patched to use the working dir
expected_output_manifest_path = (
f"{work_dir_stem}/{cleaned_pipeline_name}/{expected_run_id}"
f"{str(fn)}/{cleaned_pipeline_name}/{expected_run_id}"
f"/{component_name}/manifest.json"
)

Expand Down

0 comments on commit c8aa2b2

Please sign in to comment.