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

Use watchdog timeout to catalog properties #48

Merged
merged 4 commits into from
Sep 19, 2024

Conversation

ericvergnaud
Copy link
Contributor

@ericvergnaud ericvergnaud commented Sep 18, 2024

Changes

Current implementation does not mark catalogs as being in use for the watchdog to skip.
This PR fills the gap by adding a RemoveAfter entry to catalog properties

Linked issues

PR https://github.com/databrickslabs/watchdog/pull/58

Tests

Not tested

Copy link

github-actions bot commented Sep 18, 2024

This PR breaks backwards compatibility for databrickslabs/blueprint downstream. See build logs for more details.

Running from downstreams #28

Copy link

This PR breaks backwards compatibility for databrickslabs/ucx downstream. See build logs for more details.

Running from downstreams #23

Copy link

github-actions bot commented Sep 18, 2024

✅ 35/35 passed, 3 skipped, 7m52s total

Running from acceptance #69

@ericvergnaud ericvergnaud marked this pull request as draft September 18, 2024 15:07
Copy link
Collaborator

@nfx nfx left a comment

Choose a reason for hiding this comment

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

lgtm

@ericvergnaud ericvergnaud changed the title add watchdog name suffix to catalog name add watchdog timeout to catalog properties Sep 19, 2024
@ericvergnaud ericvergnaud marked this pull request as ready for review September 19, 2024 08:39
@ericvergnaud
Copy link
Contributor Author

@nfx following discussion with @asnare, switched to using properties instead of name suffix

@asnare
Copy link
Contributor

asnare commented Sep 19, 2024

@nfx following discussion with @asnare, switched to using properties instead of name suffix

Context: review conversations from databrickslabs/ucx#2388

@nfx nfx changed the title add watchdog timeout to catalog properties Use watchdog timeout to catalog properties Sep 19, 2024
@nfx nfx merged commit d7fb040 into main Sep 19, 2024
8 of 9 checks passed
@nfx nfx deleted the use-watchdog-name-suffix-for-catalogs branch September 19, 2024 10:44
nfx added a commit that referenced this pull request Sep 19, 2024
* Use watchdog timeout to catalog properties ([#48](#48)). This pull request introduces a new `RemoveAfter` property for catalogs, which allows for marking them for skipping by the watchdog. This change addresses the current implementation gap, which does not explicitly indicate when catalogs are being used. The new property will specify the time from which objects can be purged. A corresponding fixture `watchdog_remove_after` has been added to the list of available fixtures, and the `make_catalog` fixture has been updated to include this new property. Additionally, a timeout mechanism for catalogs has been implemented, which improves the system's efficiency and safety by marking catalogs as in use. A test for the `make_catalog` function has been included to ensure that the `RemoveAfter` entry is correctly added to the catalog properties. However, the specific call parameters for the `catalogs.create` method cannot be accurately determined in the test.
* use tags instead of name suffix for queries ([#47](#47)). This release introduces updates to the testing library for Databricks, enhancing the naming conventions for queries to improve readability and comprehension. The previous implementation used name suffixes, which have been replaced with watchdog query tags. The `watchdog_purge_suffix` fixture has been renamed to `watchdog_remove_after`, and the new `make_query` fixture has been added to the documentation. In addition, the `make_query` and `create` functions now accept an optional `tags` argument, and the query name is generated with a unique identifier. If `tags` are provided, the `RemoveAfter` tag is added. The `original_query_tag` is no longer hardcoded in the `create` function and has been removed. These changes improve the overall user experience and maintainability of the project.
@nfx nfx mentioned this pull request Sep 19, 2024
nfx added a commit that referenced this pull request Sep 19, 2024
* Use watchdog timeout to catalog properties
([#48](#48)). This pull
request introduces a new `RemoveAfter` property for catalogs, which
allows for marking them for skipping by the watchdog. This change
addresses the current implementation gap, which does not explicitly
indicate when catalogs are being used. The new property will specify the
time from which objects can be purged. A corresponding fixture
`watchdog_remove_after` has been added to the list of available
fixtures, and the `make_catalog` fixture has been updated to include
this new property. Additionally, a timeout mechanism for catalogs has
been implemented, which improves the system's efficiency and safety by
marking catalogs as in use. A test for the `make_catalog` function has
been included to ensure that the `RemoveAfter` entry is correctly added
to the catalog properties. However, the specific call parameters for the
`catalogs.create` method cannot be accurately determined in the test.
* use tags instead of name suffix for queries
([#47](#47)). This
release introduces updates to the testing library for Databricks,
enhancing the naming conventions for queries to improve readability and
comprehension. The previous implementation used name suffixes, which
have been replaced with watchdog query tags. The `watchdog_purge_suffix`
fixture has been renamed to `watchdog_remove_after`, and the new
`make_query` fixture has been added to the documentation. In addition,
the `make_query` and `create` functions now accept an optional `tags`
argument, and the query name is generated with a unique identifier. If
`tags` are provided, the `RemoveAfter` tag is added. The
`original_query_tag` is no longer hardcoded in the `create` function and
has been removed. These changes improve the overall user experience and
maintainability of the project.
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.

3 participants