Add PyDABs support for SQL Alerts - #6335
Merged
Merged
Conversation
Sankalp-Mittal
changed the title
Sankalp-Mittal
commented
Aug 20, 2026
| @@ -0,0 +1,19 @@ | |||
| from enum import Enum | |||
Collaborator
Author
There was a problem hiding this comment.
This entire directory _models/ is autogenerated
Contributor
There was a problem hiding this comment.
Are they auto-generated one-off or every time in CI?
Let's mark generated files as such:
- header comment
- change .gitattributes
- Taskfile dependency if not already
Sankalp-Mittal
commented
Aug 20, 2026
| @@ -0,0 +1,123 @@ | |||
| __all__ = [ | |||
Collaborator
Author
There was a problem hiding this comment.
This file is also auto generated
Contributor
There was a problem hiding this comment.
ditto, mark the whole python/databricks/bundles/alerts/* as auto-gen'ed
Sankalp-Mittal
commented
Aug 20, 2026
| @@ -12,6 +12,7 @@ | |||
| "VariableOrDict", | |||
Collaborator
Author
There was a problem hiding this comment.
These had to be manually added to add mutators
Sankalp-Mittal
commented
Aug 20, 2026
| @@ -69,6 +70,38 @@ def my_job_mutator(bundle: Bundle, job: Job) -> Job: | |||
| # was deemed overly implicit and potentially confusing. | |||
|
|
|||
|
|
|||
Collaborator
Author
There was a problem hiding this comment.
Again manually added but standard template code
Sankalp-Mittal
commented
Aug 20, 2026
| @@ -31,6 +31,7 @@ def all(cls) -> tuple["_ResourceType", ...]: | |||
| # intentionally lazily load all resource types to avoid imports from databricks.bundles.core to | |||
| # be imported in databricks.bundles.<resource_type> | |||
|
|
|||
Collaborator
Author
There was a problem hiding this comment.
This file should also be autogenerated we should just have a cli interface where singular and plural can be defined
Contributor
There was a problem hiding this comment.
This is duplicate from resources in go - we should only have one source of truth
Sankalp-Mittal
commented
Aug 20, 2026
| @@ -249,6 +258,34 @@ def add_volume( | |||
|
|
|||
| self._volumes[resource_name] = volume | |||
|
|
|||
| def add_alert( | |||
Collaborator
Author
There was a problem hiding this comment.
This is the only function that I'm not sure on whether to autogenerate or not??
Contributor
There was a problem hiding this comment.
looks boilerplate compared to the other resources
Sankalp-Mittal
commented
Aug 20, 2026
| @@ -74,6 +83,40 @@ class TestCase: | |||
| ), | |||
| resource_types[Schema], | |||
| ), | |||
| ( | |||
Collaborator
Author
There was a problem hiding this comment.
This is the unit test params that I think need to be manually written
Contributor
There was a problem hiding this comment.
You should be able to procedurally generate these test cases, or at least use the ones from invariant tests as starting point and transform them to the python tests
Sankalp-Mittal
commented
Aug 20, 2026
| @@ -26,6 +34,7 @@ class TestCase: | |||
| dict_example: dict | |||
| dataclass_example: Resource | |||
| mutator: Callable | |||
| article: str = "a" # grammatical article in the duplicate-resource error message | |||
Collaborator
Author
There was a problem hiding this comment.
Default article can be overriden for example in the case of alert
Sankalp-Mittal
commented
Aug 20, 2026
| @@ -21,6 +22,8 @@ | |||
| "pipelines", | |||
| "resources", | |||
| "catalog", | |||
| "sql", | |||
Collaborator
Author
There was a problem hiding this comment.
These 2 had to be added manually but I think they can be found out using BFS and potentially stored in a yaml file
Contributor
There was a problem hiding this comment.
As discussed, as follow-up: Consider inverting LOADED_NAMESPACES to always load all except the ~2 problematic ones
Collaborator
Integration test reportCommit: 0824197
Top 3 slowest tests (at least 2 minutes):
|
Sankalp-Mittal
marked this pull request as ready for review
August 21, 2026 12:43
janniklasrose
approved these changes
Aug 27, 2026
janniklasrose
left a comment
janniklasrose
left a comment
Contributor
There was a problem hiding this comment.
good to merge. consider all comments for follow-up work
| @@ -0,0 +1,19 @@ | |||
| from enum import Enum | |||
Contributor
There was a problem hiding this comment.
Are they auto-generated one-off or every time in CI?
Let's mark generated files as such:
- header comment
- change .gitattributes
- Taskfile dependency if not already
| @@ -21,6 +22,8 @@ | |||
| "pipelines", | |||
| "resources", | |||
| "catalog", | |||
| "sql", | |||
Contributor
There was a problem hiding this comment.
As discussed, as follow-up: Consider inverting LOADED_NAMESPACES to always load all except the ~2 problematic ones
| @@ -0,0 +1,123 @@ | |||
| __all__ = [ | |||
Contributor
There was a problem hiding this comment.
ditto, mark the whole python/databricks/bundles/alerts/* as auto-gen'ed
| @@ -31,6 +31,7 @@ def all(cls) -> tuple["_ResourceType", ...]: | |||
| # intentionally lazily load all resource types to avoid imports from databricks.bundles.core to | |||
| # be imported in databricks.bundles.<resource_type> | |||
|
|
|||
Contributor
There was a problem hiding this comment.
This is duplicate from resources in go - we should only have one source of truth
| @@ -249,6 +258,34 @@ def add_volume( | |||
|
|
|||
| self._volumes[resource_name] = volume | |||
|
|
|||
| def add_alert( | |||
Contributor
There was a problem hiding this comment.
looks boilerplate compared to the other resources
|
|
||
| if self._alerts.get(resource_name): | ||
| self.add_diagnostic_error( | ||
| msg=f"Duplicate resource name '{resource_name}' for an alert. Resource names must be unique.", |
Contributor
There was a problem hiding this comment.
to avoid the test refactoring below, I'd suggest using different wording for all errors to not have to worry about this (or add article to _ResourceType)
Suggested change
| msg=f"Duplicate resource name '{resource_name}' for an alert. Resource names must be unique.", | |
| msg=f"Duplicate resource name '{resource_name}' for resource 'alert'. Resource names must be unique.", |
| @@ -74,6 +83,40 @@ class TestCase: | |||
| ), | |||
| resource_types[Schema], | |||
| ), | |||
| ( | |||
Contributor
There was a problem hiding this comment.
You should be able to procedurally generate these test cases, or at least use the ones from invariant tests as starting point and transform them to the python tests
Comment on lines
+6
to
+10
| python: | ||
| resources: | ||
| - "resources:load_resources" | ||
| mutators: | ||
| - "mutators:update_alert" |
Contributor
There was a problem hiding this comment.
for now ok, but let's make sure when we do full auto-gen that we test the conflict keys etc from test_resources.py in acceptance tests for more confidence
Sankalp-Mittal
enabled auto-merge
August 27, 2026 09:43
Sankalp-Mittal
force-pushed
the
sankalp-mittal/pydabs-support-for-alerts
branch
from
August 27, 2026 09:45
43bc6f1 to
0824197
Compare
Collaborator
Integration test reportCommit: 5766a14
13 interesting tests: 4 flaky, 4 FAIL, 2 RECOVERED, 2 SKIP, 1 KNOWN
Top 50 slowest tests (at least 2 minutes):
|
Sankalp-Mittal
added a commit
that referenced
this pull request
Aug 28, 2026
The PyDABs codegen writes databricks/bundles/<ns>/_models/*.py and each namespace __init__.py, but those files carried no generated-file marker. - Prepend a "# Code generated by pydabs-codegen. DO NOT EDIT." header in both writer paths (_write_code, _write_exports) and regenerate. - Add a nested python/databricks/bundles/.gitattributes marking the generated _models trees and namespace __init__.py files as linguist-generated (core/ and resources/ stay hand-written). Addresses review comments on #6335. Co-authored-by: Isaac <no-reply@databricks.com>
iammuntazirali
pushed a commit
to iammuntazirali/cli
that referenced
this pull request
Aug 29, 2026
Stacked on databricks#6393. The PyDABs codegen (`python/codegen`) writes `databricks/bundles/<ns>/_models/*.py` and each namespace `__init__.py`, but those files carried no generated-file marker. This addresses review comments on databricks#6335 asking to mark generated files as such. ## Changes - **Header comment:** prepend `# Code generated by pydabs-codegen. DO NOT EDIT.` in both writer paths (`_write_code`, `_write_exports`) in `python/codegen/codegen/main.py`, then regenerate. The regeneration diff is exactly the header (+2 lines per file) — nothing else moves. - **`.gitattributes`:** add a nested `python/databricks/bundles/.gitattributes` marking the generated `_models` trees and the six generated namespace `__init__.py` files (alerts, catalogs, jobs, pipelines, schemas, volumes) as `linguist-generated`. `core/` and `resources/` are hand-written and stay unmarked. ## Verification - `git check-attr` confirms generated files are marked, hand-written files are not. - `codegen_tests`: 8 passed; `pydabs-test`: 220 passed. - `ruff format` + `check` clean on `main.py`. This pull request and its description were written by Isaac. --------- Co-authored-by: Isaac <no-reply@databricks.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
3 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
This PR adds support for using alerts via Python
Changes
sqlandiamnamespace (packages.py), wire Alert into the core package (add_alert, alerts collection, alert_mutator, resource-type registration), and commit the generated databricks/bundles/alerts/ package.Why
This change was requested by a PyDABs user, and is part of eventually supporting all resources in PyDABs
Tests