Skip to content

Commit

Permalink
Fix mount permissions on input volumes (#492)
Browse files Browse the repository at this point in the history
* Add Data Preparator cookiecutter template

* Rename cookiecutter folder

* Temporarily remove possibly offending files

* Remove cookicutter conditionals

* Inclube back missing pieces of template

* remove cookiecutter typo

* Use project_name attribute

* Change cookiecutter fields order

* Create empty directories on hook

* Fix empty folders paths

* Create evaluator mlcube cookiecutter template

* Fix JSON Syntax Error

* Update template default values

* Remove reference to undefined template variable

* Implement model mlcube cookiecutter template

* Update cookiecutter variable default values

* Create medperf CLI command for creating MLCubes

* Provide additional options for mlcube create

* Start working on tests

* Add tests for cube create

* Ignore invalid syntax on cookiecutter conditionals

* Ignore more flake8 errors

* Remove unused import

* Empty commit for cloudbuild

* Fix inconsistency with labels paths

* Update mlcube.yaml so it can be commented on docs

* Don't render noqa comments on template

* Remove flake8 specific ignores

* Exclude templates from lint checks

* Remove specific flake8 ignores

* Fix labels_paht being passed in he wrong situation

* Add requirements to cookiecutters

* Set separate labels as true by default

* Remove duplicate templates

* Abstract field-error dict formatting

* Reformat errors dictionary for printing

* Implement general read-only input permission

* Remove unnecessary permission definition

* Fix/implement ro tests

---------

Co-authored-by: hasan7n <[email protected]>
  • Loading branch information
aristizabal95 and hasan7n authored Dec 1, 2023
1 parent 5a6a1e3 commit de0ec67
Show file tree
Hide file tree
Showing 6 changed files with 22 additions and 31 deletions.
13 changes: 0 additions & 13 deletions cli/medperf/commands/dataset/create.py
Original file line number Diff line number Diff line change
Expand Up @@ -119,33 +119,22 @@ def run_cube_tasks(self):
"output_path": out_datapath,
"output_labels_path": out_labelspath,
}
prepare_str_params = {
"Ptasks.prepare.parameters.input.data_path.opts": "ro",
"Ptasks.prepare.parameters.input.labels_path.opts": "ro",
}

sanity_params = {
"data_path": out_datapath,
"labels_path": out_labelspath,
}
sanity_str_params = {
"Ptasks.sanity_check.parameters.input.data_path.opts": "ro"
}

statistics_params = {
"data_path": out_datapath,
"output_path": self.out_statistics_path,
"labels_path": out_labelspath,
}
statistics_str_params = {
"Ptasks.statistics.parameters.input.data_path.opts": "ro"
}

# Run the tasks
self.ui.text = "Running preparation step..."
self.cube.run(
task="prepare",
string_params=prepare_str_params,
timeout=prepare_timeout,
**prepare_params,
)
Expand All @@ -154,7 +143,6 @@ def run_cube_tasks(self):
self.ui.text = "Running sanity check..."
self.cube.run(
task="sanity_check",
string_params=sanity_str_params,
timeout=sanity_check_timeout,
**sanity_params,
)
Expand All @@ -163,7 +151,6 @@ def run_cube_tasks(self):
self.ui.text = "Generating statistics..."
self.cube.run(
task="statistics",
string_params=statistics_str_params,
timeout=statistics_timeout,
**statistics_params,
)
Expand Down
5 changes: 0 additions & 5 deletions cli/medperf/commands/execution.py
Original file line number Diff line number Diff line change
Expand Up @@ -86,7 +86,6 @@ def run_inference(self):
timeout=infer_timeout,
data_path=data_path,
output_path=preds_path,
string_params={"Ptasks.infer.parameters.input.data_path.opts": "ro"},
)
self.ui.print("> Model execution complete")

Expand All @@ -113,10 +112,6 @@ def run_evaluation(self):
predictions=preds_path,
labels=labels_path,
output_path=results_path,
string_params={
"Ptasks.evaluate.parameters.input.predictions.opts": "ro",
"Ptasks.evaluate.parameters.input.labels.opts": "ro",
},
)
except ExecutionError as e:
logging.error(f"Metrics MLCube Execution failed: {e}")
Expand Down
4 changes: 4 additions & 0 deletions cli/medperf/entities/cube.py
Original file line number Diff line number Diff line change
Expand Up @@ -277,6 +277,7 @@ def run(
output_logs: str = None,
string_params: Dict[str, str] = {},
timeout: int = None,
read_protected_input: bool = True,
**kwargs,
):
"""Executes a given task on the cube instance
Expand All @@ -286,6 +287,7 @@ def run(
string_params (Dict[str], optional): Extra parameters that can't be passed as normal function args.
Defaults to {}.
timeout (int, optional): timeout for the task in seconds. Defaults to None.
read_protected_input (bool, optional): Wether to disable write permissions on input volumes. Defaults to True.
kwargs (dict): additional arguments that are passed directly to the mlcube command
"""
kwargs.update(string_params)
Expand All @@ -294,6 +296,8 @@ def run(
cmd += f" --mlcube={self.cube_path} --task={task} --platform={config.platform} --network=none"
if config.gpus is not None:
cmd += f" --gpus={config.gpus}"
if read_protected_input:
cmd += " --mount=ro"
for k, v in kwargs.items():
cmd_arg = f'{k}="{v}"'
cmd = " ".join([cmd, cmd_arg])
Expand Down
6 changes: 0 additions & 6 deletions cli/medperf/tests/commands/dataset/test_create.py
Original file line number Diff line number Diff line change
Expand Up @@ -116,25 +116,19 @@ def test_run_cube_tasks_runs_required_tasks(self, mocker, preparation):
labels_path=LABELS_PATH,
output_path=OUT_DATAPATH,
output_labels_path=OUT_LABELSPATH,
string_params={
"Ptasks.prepare.parameters.input.data_path.opts": "ro",
"Ptasks.prepare.parameters.input.labels_path.opts": "ro",
},
)
check = call(
task="sanity_check",
timeout=None,
data_path=OUT_DATAPATH,
labels_path=OUT_LABELSPATH,
string_params={"Ptasks.sanity_check.parameters.input.data_path.opts": "ro"},
)
stats = call(
task="statistics",
timeout=None,
data_path=OUT_DATAPATH,
labels_path=OUT_LABELSPATH,
output_path=STATISTICS_PATH,
string_params={"Ptasks.statistics.parameters.input.data_path.opts": "ro"},
)
calls = [prepare, check, stats]

Expand Down
5 changes: 0 additions & 5 deletions cli/medperf/tests/commands/test_execution.py
Original file line number Diff line number Diff line change
Expand Up @@ -178,7 +178,6 @@ def test_cube_run_are_called_properly(mocker, setup):
timeout=config.infer_timeout,
data_path=INPUT_DATASET.data_path,
output_path=exp_preds_path,
string_params={"Ptasks.infer.parameters.input.data_path.opts": "ro"},
)
exp_eval_call = call(
task="evaluate",
Expand All @@ -187,10 +186,6 @@ def test_cube_run_are_called_properly(mocker, setup):
predictions=exp_preds_path,
labels=INPUT_DATASET.labels_path,
output_path=ANY,
string_params={
"Ptasks.evaluate.parameters.input.predictions.opts": "ro",
"Ptasks.evaluate.parameters.input.labels.opts": "ro",
},
)
# Act
Execution.run(INPUT_DATASET, INPUT_MODEL, INPUT_EVALUATOR)
Expand Down
20 changes: 18 additions & 2 deletions cli/medperf/tests/entities/test_cube.py
Original file line number Diff line number Diff line change
Expand Up @@ -162,7 +162,7 @@ def test_cube_runs_command(self, mocker, timeout, setup, task):
)
expected_cmd = (
f"mlcube run --mlcube={self.manifest_path} --task={task} "
+ f"--platform={self.platform} --network=none"
+ f"--platform={self.platform} --network=none --mount=ro"
)

# Act
Expand All @@ -172,13 +172,29 @@ def test_cube_runs_command(self, mocker, timeout, setup, task):
# Assert
spy.assert_any_call(expected_cmd, timeout=timeout)

def test_cube_runs_command_with_rw_access(self, mocker, setup, task):
# Arrange
mpexpect = MockPexpect(0, "expected_hash")
spy = mocker.patch("pexpect.spawn", side_effect=mpexpect.spawn)
expected_cmd = (
f"mlcube run --mlcube={self.manifest_path} --task={task} "
+ f"--platform={self.platform} --network=none"
)

# Act
cube = Cube.get(self.id)
cube.run(task, read_protected_input=False)

# Assert
spy.assert_any_call(expected_cmd, timeout=None)

def test_cube_runs_command_with_extra_args(self, mocker, setup, task):
# Arrange
mpexpect = MockPexpect(0, "expected_hash")
spy = mocker.patch("pexpect.spawn", side_effect=mpexpect.spawn)
expected_cmd = (
f"mlcube run --mlcube={self.manifest_path} --task={task} "
+ f'--platform={self.platform} --network=none test="test"'
+ f'--platform={self.platform} --network=none --mount=ro test="test"'
)

# Act
Expand Down

0 comments on commit de0ec67

Please sign in to comment.