Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Pause quality monitors when "mode: development" is used #1481

Merged

Conversation

lennartkats-db
Copy link
Contributor

@lennartkats-db lennartkats-db commented Jun 10, 2024

Changes

Similar to scheduled jobs, quality monitors should be paused when in development mode (in line with the behavior for scheduled jobs). @aravind-segu @arpitjasa-db please take a look and verify this behavior.

Tests

Unit tests

@codecov-commenter
Copy link

codecov-commenter commented Jun 10, 2024

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 53.58%. Comparing base (e22dd8a) to head (49a873a).
Report is 138 commits behind head on main.

Additional details and impacted files
@@            Coverage Diff             @@
##             main    #1481      +/-   ##
==========================================
+ Coverage   52.25%   53.58%   +1.33%     
==========================================
  Files         317      351      +34     
  Lines       18004    20281    +2277     
==========================================
+ Hits         9408    10868    +1460     
- Misses       7903     8614     +711     
- Partials      693      799     +106     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

@arpitjasa-db arpitjasa-db requested a review from vperiyasamy June 10, 2024 07:26
@arpitjasa-db
Copy link
Contributor

@vperiyasamy to confirm behavior from Monitoring side

@lennartkats-db
Copy link
Contributor Author

Related PR, generalizing the "mode: development" behavior: #1490

Copy link

@vperiyasamy vperiyasamy left a comment

Choose a reason for hiding this comment

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

We actually don't support pausing the monitor schedules directly, the schedule status is a read-only field that we exposed because we were considering building an "auto pausing" feature which would detect successive failures. Is it required for us to have this behavior? If so, we can delete the schedule to "pause" it.

@lennartkats-db
Copy link
Contributor Author

@aravind-segu @arpitjasa-db @vperiyasamy I updated the PR to remove the schedule instead for now, please take another look.

I also updated the config test so it actually works out of the box on staging in order to verify this change.

bundle/config/mutator/process_target_mode.go Show resolved Hide resolved
@@ -24,14 +31,14 @@ targets:
quality_monitors:
my_monitor:
table_name: "main.test.staging"
output_schema_name: "staging"
output_schema_name: "main.test"

production:
resources:
quality_monitors:
my_monitor:

Choose a reason for hiding this comment

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

what does this name mean?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

The original name wasn't valid according the the API. It requires something of the format catalog.schema. Actually, that seems like something to reflect on https://docs.databricks.com/api/workspace/qualitymonitors/create?

Copy link
Contributor

@pietern pietern left a comment

Choose a reason for hiding this comment

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

Minor comment about the test configuration. Rest LGTM.

Copy link
Contributor

@arpitjasa-db arpitjasa-db left a comment

Choose a reason for hiding this comment

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

Minor question on test, otherwise LGTM

bundle/tests/quality_monitor/databricks.yml Outdated Show resolved Hide resolved
@lennartkats-db lennartkats-db enabled auto-merge June 19, 2024 13:52
@lennartkats-db lennartkats-db added this pull request to the merge queue Jun 19, 2024
Merged via the queue into databricks:main with commit deb3e36 Jun 19, 2024
5 checks passed
@lennartkats-db lennartkats-db deleted the target-mode-for-monitors branch June 19, 2024 14:00
pietern added a commit that referenced this pull request Jun 26, 2024
CLI:
 * Add link to documentation for Homebrew installation to README ([#1505](#1505)).
 * Fix `databricks configure` to use `DATABRICKS_CONFIG_FILE` environment variable if exists as config file ([#1325](#1325)).

Bundles:

The Terraform upgrade to v1.48.0 includes a fix for library order not being respected.

 * Fix conditional in query in `default-sql` template ([#1479](#1479)).
 * Remove user credentials specified in the Git origin URL ([#1494](#1494)).
 * Serialize dynamic value for `bundle validate` output ([#1499](#1499)).
 * Override variables with lookup value even if values has default value set ([#1504](#1504)).
 * Pause quality monitors when "mode: development" is used ([#1481](#1481)).
 * Return `fs.ModeDir` for Git folders in the workspace ([#1521](#1521)).
 * Upgrade TF provider to 1.48.0 ([#1527](#1527)).
 * Added support for complex variables ([#1467](#1467)).

Internal:
 * Add randIntn function ([#1475](#1475)).
 * Avoid multiple file tree traversals on bundle deploy ([#1493](#1493)).
 * Clean up unused code ([#1502](#1502)).
 * Use `dyn.InvalidValue` to indicate absence ([#1507](#1507)).
 * Add ApplyPythonMutator ([#1430](#1430)).
 * Set bool pointer to disable lock ([#1516](#1516)).
 * Allow the any type to be set to nil in `convert.FromTyped` ([#1518](#1518)).
 * Properly deal with nil values in `convert.FromTyped` ([#1511](#1511)).
 * Return `dyn.InvalidValue` instead of `dyn.NilValue` when errors happen ([#1514](#1514)).
 * PythonMutator: replace stdin/stdout with files ([#1512](#1512)).
 * Add context type and value to path rewriting ([#1525](#1525)).

API Changes:
 * Added schedule CRUD commands to `databricks lakeview`.
 * Added subscription CRUD commands to `databricks lakeview`.
 * Added `databricks apps start` command.

OpenAPI commit 7437dabb9dadee402c1fc060df4c1ce8cc5369f0 (2024-06-24)

Dependency updates:
 * Bump golang.org/x/text from 0.15.0 to 0.16.0 ([#1482](#1482)).
 * Bump golang.org/x/term from 0.20.0 to 0.21.0 ([#1483](#1483)).
 * Bump golang.org/x/mod from 0.17.0 to 0.18.0 ([#1484](#1484)).
 * Bump golang.org/x/oauth2 from 0.20.0 to 0.21.0 ([#1485](#1485)).
 * Bump github.com/briandowns/spinner from 1.23.0 to 1.23.1 ([#1495](#1495)).
 * Bump github.com/spf13/cobra from 1.8.0 to 1.8.1 ([#1496](#1496)).
 * Bump github.com/databricks/databricks-sdk-go from 0.42.0 to 0.43.0 ([#1522](#1522)).
@pietern pietern mentioned this pull request Jun 26, 2024
github-merge-queue bot pushed a commit that referenced this pull request Jun 26, 2024
CLI:
* Add link to documentation for Homebrew installation to README
([#1505](#1505)).
* Fix `databricks configure` to use `DATABRICKS_CONFIG_FILE` environment
variable if exists as config file
([#1325](#1325)).

Bundles:

The Terraform upgrade to v1.48.0 includes a fix for library order not
being respected.

* Fix conditional in query in `default-sql` template
([#1479](#1479)).
* Remove user credentials specified in the Git origin URL
([#1494](#1494)).
* Serialize dynamic value for `bundle validate` output
([#1499](#1499)).
* Override variables with lookup value even if values has default value
set ([#1504](#1504)).
* Pause quality monitors when "mode: development" is used
([#1481](#1481)).
* Return `fs.ModeDir` for Git folders in the workspace
([#1521](#1521)).
* Upgrade TF provider to 1.48.0
([#1527](#1527)).
* Added support for complex variables
([#1467](#1467)).

Internal:
* Add randIntn function
([#1475](#1475)).
* Avoid multiple file tree traversals on bundle deploy
([#1493](#1493)).
* Clean up unused code
([#1502](#1502)).
* Use `dyn.InvalidValue` to indicate absence
([#1507](#1507)).
* Add ApplyPythonMutator
([#1430](#1430)).
* Set bool pointer to disable lock
([#1516](#1516)).
* Allow the any type to be set to nil in `convert.FromTyped`
([#1518](#1518)).
* Properly deal with nil values in `convert.FromTyped`
([#1511](#1511)).
* Return `dyn.InvalidValue` instead of `dyn.NilValue` when errors happen
([#1514](#1514)).
* PythonMutator: replace stdin/stdout with files
([#1512](#1512)).
* Add context type and value to path rewriting
([#1525](#1525)).

API Changes:
 * Added schedule CRUD commands to `databricks lakeview`.
 * Added subscription CRUD commands to `databricks lakeview`.
 * Added `databricks apps start` command.

OpenAPI commit 7437dabb9dadee402c1fc060df4c1ce8cc5369f0 (2024-06-24)

Dependency updates:
* Bump golang.org/x/text from 0.15.0 to 0.16.0
([#1482](#1482)).
* Bump golang.org/x/term from 0.20.0 to 0.21.0
([#1483](#1483)).
* Bump golang.org/x/mod from 0.17.0 to 0.18.0
([#1484](#1484)).
* Bump golang.org/x/oauth2 from 0.20.0 to 0.21.0
([#1485](#1485)).
* Bump github.com/briandowns/spinner from 1.23.0 to 1.23.1
([#1495](#1495)).
* Bump github.com/spf13/cobra from 1.8.0 to 1.8.1
([#1496](#1496)).
* Bump github.com/databricks/databricks-sdk-go from 0.42.0 to 0.43.0
([#1522](#1522)).
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants