Skip to content

Commit

Permalink
sanitise (#108)
Browse files Browse the repository at this point in the history
Co-authored-by: Maxim Mityutko <[email protected]>
  • Loading branch information
maxim-mityutko and Maxim Mityutko authored Apr 19, 2024
1 parent 35d0824 commit 2450bf4
Show file tree
Hide file tree
Showing 2 changed files with 41 additions and 5 deletions.
22 changes: 21 additions & 1 deletion brickflow/engine/task.py
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,7 @@
import dataclasses
import functools
import inspect
import json
import logging
import textwrap
from dataclasses import dataclass, field
Expand Down Expand Up @@ -681,7 +682,26 @@ def _skip_because_not_selected(self) -> Tuple[bool, Optional[str]]:
)
if selected_tasks is None or selected_tasks == "":
return False, None
selected_task_list = selected_tasks.split(",")

if selected_tasks.startswith("[") and selected_tasks.endswith("]"):
try:
selected_task_list = json.loads(selected_tasks)
except json.JSONDecodeError:
selected_task_list = []
_ilog.info(
"Invalid JSON list in `brickflow_internal_only_run_tasks` parameter. Nothing will be skipped."
)
except Exception as e:
selected_task_list = []
_ilog.info(
"Error parsing `brickflow_internal_only_run_tasks` parameter as JSON, nothing to skip. Error: %s",
str(e),
)
else:
selected_task_list = selected_tasks.split(",")

selected_task_list = [task.strip() for task in selected_task_list]

if self.name not in selected_task_list:
return (
True,
Expand Down
24 changes: 20 additions & 4 deletions tests/engine/test_task.py
Original file line number Diff line number Diff line change
Expand Up @@ -174,9 +174,16 @@ def test_should_skip_false(self, task_coms_mock: Mock):
assert reason is None
ctx._configure()

@pytest.mark.parametrize(
"tasks",
[
"somethingelse", # other task
f"[{task_function_4.__name__}]", # invalid JSON list defaults to no skip
],
)
@patch("brickflow.context.ctx.get_parameter")
def test_skip_not_selected_task(self, dbutils):
dbutils.value = "sometihngelse"
def test_skip_not_selected_task(self, dbutils, tasks):
dbutils.return_value = tasks
skip, reason = wf.get_task(
task_function_4.__name__
)._skip_because_not_selected()
Expand All @@ -189,9 +196,18 @@ def test_skip_not_selected_task(self, dbutils):
)
assert wf.get_task(task_function_4.__name__).execute() is None

@pytest.mark.parametrize(
"tasks",
[
task_function_4.__name__, # clean string
f'["{task_function_4.__name__}"]', # clean JSON list
f'[" {task_function_4.__name__} "]', # spaced JSON list
f" {task_function_4.__name__} ", # spaced string
],
)
@patch("brickflow.context.ctx.get_parameter")
def test_no_skip_selected_task(self, dbutils: Mock):
dbutils.return_value = task_function_4.__name__
def test_no_skip_selected_task(self, dbutils, tasks):
dbutils.return_value = tasks
skip, reason = wf.get_task(
task_function_4.__name__
)._skip_because_not_selected()
Expand Down

0 comments on commit 2450bf4

Please sign in to comment.