diff --git a/experimental/python/databricks/bundles/build.py b/experimental/python/databricks/bundles/build.py index 3deadff2f6..86c9d1f7c7 100644 --- a/experimental/python/databricks/bundles/build.py +++ b/experimental/python/databricks/bundles/build.py @@ -166,16 +166,12 @@ def _apply_mutators_for_type( return resources, Diagnostics() -def python_mutator( - args: _Args, -) -> tuple[dict, dict[tuple[str, ...], Location], Diagnostics]: - input = json.load(open(args.input, encoding="utf-8")) +def _read_conf(input: dict) -> tuple[_Conf, Diagnostics]: experimental = input.get("experimental", {}) if experimental.get("pydabs", {}) != {}: return ( - {}, - {}, + _Conf(), Diagnostics.create_error( "'experimental/pydabs' is not supported by 'databricks-bundles', use 'experimental/python' instead", detail="", @@ -184,8 +180,53 @@ def python_mutator( ), ) - conf_dict = experimental.get("python", {}) + experimental_conf_dict = experimental.get("python", {}) + experimental_conf = _transform(_Conf, experimental_conf_dict) + + conf_dict = input.get("python", {}) conf = _transform(_Conf, conf_dict) + + has_conf = conf != _Conf() + has_experimental_conf = experimental_conf != _Conf() + + if has_conf and not has_experimental_conf: + return conf, Diagnostics() + elif not has_conf and has_experimental_conf: + # do not generate warning in Python code, if CLI supports non-experimental 'python', + # it should generate a warning + return experimental_conf, Diagnostics() + elif has_conf and has_experimental_conf: + # for backward-compatibility, CLI can copy contents of 'python' into 'experimental/python' + # if configs are equal, it isn't a problem + if conf != experimental_conf: + return ( + _Conf(), + Diagnostics.create_error( + "Both 'python' and 'experimental/python' sections are present, use 'python' section only", + detail="", + location=None, + path=( + "experimental", + "python", + ), + ), + ) + else: + return conf, Diagnostics() + else: + return _Conf(), Diagnostics() + + +def python_mutator( + args: _Args, +) -> tuple[dict, dict[tuple[str, ...], Location], Diagnostics]: + input = json.load(open(args.input, encoding="utf-8")) + diagnostics = Diagnostics() + + conf, diagnostics = diagnostics.extend_tuple(_read_conf(input)) + if diagnostics.has_error(): + return input, {}, diagnostics + bundle = _parse_bundle_info(input) if args.phase == "load_resources": diff --git a/experimental/python/databricks/bundles/core/_diagnostics.py b/experimental/python/databricks/bundles/core/_diagnostics.py index e1da027801..f53efae551 100644 --- a/experimental/python/databricks/bundles/core/_diagnostics.py +++ b/experimental/python/databricks/bundles/core/_diagnostics.py @@ -134,6 +134,17 @@ def has_error(self) -> bool: return False + def has_warning(self) -> bool: + """ + Returns True if there is at least one warning in diagnostics. + """ + + for item in self.items: + if item.severity == Severity.WARNING: + return True + + return False + @classmethod def create_error( cls, diff --git a/experimental/python/databricks_tests/core/test_diagnostics.py b/experimental/python/databricks_tests/core/test_diagnostics.py index 69f13a7390..79fd150e1a 100644 --- a/experimental/python/databricks_tests/core/test_diagnostics.py +++ b/experimental/python/databricks_tests/core/test_diagnostics.py @@ -85,3 +85,17 @@ def test_location_from_weird_callable(): location = Location.from_callable(print) assert location is None + + +def test_has_error(): + diagnostics = Diagnostics.create_error("foo is deprecated") + + assert diagnostics.has_error() + assert not diagnostics.has_warning() + + +def test_has_warning(): + diagnostics = Diagnostics.create_warning("foo is deprecated") + + assert not diagnostics.has_error() + assert diagnostics.has_warning() diff --git a/experimental/python/databricks_tests/test_build.py b/experimental/python/databricks_tests/test_build.py index c0d57ff6a9..6906adbcac 100644 --- a/experimental/python/databricks_tests/test_build.py +++ b/experimental/python/databricks_tests/test_build.py @@ -13,6 +13,7 @@ _load_resources_from_input, _parse_args, _parse_bundle_info, + _read_conf, _relativize_location, _write_diagnostics, _write_locations, @@ -487,3 +488,99 @@ def load_resources_2() -> Resources: file="my_job_2.py", line=42, column=1 ), } + + +def test_parse_conf_empty(): + actual, diagnostics = _read_conf({}) + + assert actual == _Conf() + assert diagnostics == Diagnostics() + + +def test_parse_conf_both_equal(): + conf, diagnostics = _read_conf( + { + "python": { + "venv_path": "venv", + "resources": ["my_package:load_resources"], + "mutators": ["my_package:mutator_1"], + }, + "experimental": { + "python": { + "venv_path": "venv", + "resources": ["my_package:load_resources"], + "mutators": ["my_package:mutator_1"], + }, + }, + } + ) + + assert not diagnostics.has_warning() + assert not diagnostics.has_error() + assert conf == _Conf( + venv_path="venv", + resources=["my_package:load_resources"], + mutators=["my_package:mutator_1"], + ) + + +def test_parse_conf_both_incompatible(): + _, diagnostics = _read_conf( + { + "python": { + "venv_path": "venv", + "resources": ["my_package:load_resources"], + "mutators": ["my_package:mutator_1"], + }, + "experimental": { + "python": { + "venv_path": "venv", + "resources": ["my_package:load_resources"], + }, + }, + } + ) + + assert not diagnostics.has_warning() + assert diagnostics.has_error() + + +def test_parse_conf_experimental(): + conf, diagnostics = _read_conf( + { + "experimental": { + "python": { + "venv_path": "venv", + "resources": ["my_package:load_resources"], + "mutators": ["my_package:mutator_1"], + }, + }, + } + ) + + assert not diagnostics.has_warning() + assert not diagnostics.has_error() + assert conf == _Conf( + venv_path="venv", + resources=["my_package:load_resources"], + mutators=["my_package:mutator_1"], + ) + + +def test_parse_conf(): + conf, diagnostics = _read_conf( + { + "python": { + "venv_path": "venv", + "resources": ["my_package:load_resources"], + "mutators": ["my_package:mutator_1"], + }, + } + ) + + assert diagnostics == Diagnostics() + assert conf == _Conf( + venv_path="venv", + resources=["my_package:load_resources"], + mutators=["my_package:mutator_1"], + )