Skip to content

fix: pass custom_checks through to DQEngine.save_checks - #1501

Draft
GewoonMaarten wants to merge 4 commits into
databrickslabs:mainfrom
GewoonMaarten:fix/save-custom-checks
Draft

fix: pass custom_checks through to DQEngine.save_checks#1501
GewoonMaarten wants to merge 4 commits into
databrickslabs:mainfrom
GewoonMaarten:fix/save-custom-checks

Conversation

@GewoonMaarten

Copy link
Copy Markdown
Contributor

Changes

Linked issues

Resolves #1467

Tests

  • manually tested
  • added unit tests
  • added integration tests
  • added end-to-end tests
  • added performance tests

Documentation and Demos

  • added/updated demos
  • added/updated docs
  • added/updated agent skills

@github-actions

github-actions Bot commented Sep 3, 2026

Copy link
Copy Markdown
Contributor

All commits in PR should be signed ('git commit -S ...'). See https://docs.github.com/en/authentication/managing-commit-signature-verification/signing-commits

@ghanse ghanse left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Thanks for the contribution! Left a few comments. You will need to sign your commits (see Git setup).

Comment on lines +297 to +299
def _resolve_name_and_fingerprint(
original_check: dict, normalized_check: dict, custom_checks: dict[str, Callable] | None = None
) -> tuple[str | None, str]:

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Let's add a line in the Args section of the docstring for custom_checks

run_config_name: str = "default",
rule_set_fingerprint: str | None = None,
created_at: datetime | None = None,
custom_checks: dict[str, Callable] | None = None,

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Need to also add this to the Args section in the docstring.

Comment on lines +335 to +336
custom_check_functions: Optional dictionary with custom check functions (e.g., *globals()* of
the calling module).

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

I think this should be added to the ChecksStorageHandler.save method instead?

Comment on lines +981 to +986
def save(
self,
checks: list[dict],
config: LakebaseChecksStorageConfig,
custom_check_functions: dict[str, Callable] | None = None,
) -> None:

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

You will need to pass custom_check_functions through a few methods:

_save_checks_to_lakebase -> _normalize_checks

config: BaseChecksStorageConfig,
variables: dict[str, VariableValue] | None = None,
semantic_validation_mode: str | None = ChecksSemanticValidationMode.WARN,
custom_check_functions: dict[str, Callable] | None = None,

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Similar to the other methods, this argument needs a line in the method docstring.

checks: list[dict],
config: InstallationChecksStorageConfig,
custom_check_functions: dict[str, Callable] | None = None,
) -> None:

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Need to pass customer_check_functions to handler.save here.

@ghanse ghanse added under-review This PR is currently being reviewed by one of DQX maintainers. needs-changes Changes required after review labels Sep 4, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

needs-changes Changes required after review under-review This PR is currently being reviewed by one of DQX maintainers.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

save_checks does not work with custom functions

2 participants