[Python] Support non-experimental conf in Python code #3539
Conversation
kanterov
temporarily deployed
to
test-trigger-is
GitHub Actions
Inactive
9 failing tests:
|
kanterov
requested review from
andrewnester,
anton-107,
denik,
pietern and
shreyas-goenka
as code owners
| return ( | ||
| _Conf(), | ||
| Diagnostics.create_error( | ||
| "Both 'python' and 'experimental/python' sections are present, use 'python' section only", |
There was a problem hiding this comment.
Why not have validation in CLI itself and keep Python part light? E.g. use 'python', fallback to 'experimental.python'.
There was a problem hiding this comment.
There is CLI validation in #3540, but we can't assume that both have the latest version. So we are defensive on both ends.
There was a problem hiding this comment.
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?
There was a problem hiding this comment.
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
There was a problem hiding this comment.
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.
pietern
left a comment
pietern
left a comment
There was a problem hiding this comment.
The change itself looks good.
Let's continue the discussion about the merits of version checks elsewhere.
## Changes Move `python` outside of the `experimental` section. The change is backward-compatible and forward-compatible. If CLI is using a newer or older version of `databricks-bundles,` it will continue to work. It's implemented by copying the content of `python` into `experimental.python`. If we don't offer compatibility, there is a risk that previously deployed resources will be erased because we wouldn't be able to load resources. See also #3539 ## Why Python support for Databricks Asset Bundles is graduating from the experimental status. ## Tests Acceptance tests check that previous `databricks-bundles` versions continue to work.
Changes
Support reading both
pythonandexperimental.pythonconfigs. If both are set, their value should be equivalent.After we introduce the
pythonsection into the CLI, it has to keep setting bothpythonandexperimental.pythoninto the payload for backward compatibility. After that, Python code can start erroring out ifexperimental/pythonis set but notpythonbecause it means that users have an outdated CLI version.See also #3540
Why
It's a step toward graduating
pythonoutside theexperimentalsection.Tests
Unit tests and existing acceptance tests