Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
55 changes: 48 additions & 7 deletions experimental/python/databricks/bundles/build.py
Original file line number Diff line number Diff line change
Expand Up @@ -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="",
Expand All @@ -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",
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why not have validation in CLI itself and keep Python part light? E.g. use 'python', fallback to 'experimental.python'.

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

There is CLI validation in #3540, but we can't assume that both have the latest version. So we are defensive on both ends.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

There is CLI validation in #3540, but we can't assume that both have the latest version.

Why not enforce minimum or exact databricks-bundles version in CLI?

Copy link
Collaborator Author

@kanterov kanterov Sep 8, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This change is also ready to go now. We don't need to wait for the CLI release including support for non-experimental "python" section

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

For discussion: do you have ideas on how to enforce bidirectional compat for future changes? If so, do we use some kind of version range to do this? Some changes will require a coupled bump. It may be easier to just require version coupling, or at least warn if you're using different versions.

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":
Expand Down
11 changes: 11 additions & 0 deletions experimental/python/databricks/bundles/core/_diagnostics.py
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand Down
14 changes: 14 additions & 0 deletions experimental/python/databricks_tests/core/test_diagnostics.py
Original file line number Diff line number Diff line change
Expand Up @@ -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()
97 changes: 97 additions & 0 deletions experimental/python/databricks_tests/test_build.py
Original file line number Diff line number Diff line change
Expand Up @@ -13,6 +13,7 @@
_load_resources_from_input,
_parse_args,
_parse_bundle_info,
_read_conf,
_relativize_location,
_write_diagnostics,
_write_locations,
Expand Down Expand Up @@ -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"],
)
Loading