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

A method to add callbacks to the global event manager #132

Merged
merged 5 commits into from
May 15, 2024

Conversation

QMalcolm
Copy link
Contributor

resolves #131

Description

It was annoying me that we had a way to add loggers to the global event manager but not callbacks. I'm doing work in dbt-core that could benefit from this. So I decided to do the work here to give callbacks the same support as loggers.

Checklist

QMalcolm added 4 commits May 14, 2024 16:03
We did this refactor because we plan to use the `EventCatcher` in more unit test
files. We have a similar class in dbt-core currently. It might be worthwhile at a
future date to distribute this class via the `dbt_common` package itself.

Also the empty `__init__.py` files are unfortunately necessary it seems. This is
because without them mypy will complain about "Source file found twice under
different module names".
We had done this work previously by ignoring event classes that came
from modules beginning with `test_`. This worked because if we created
or re-implemented events in test files, their module was the the file
that they existed in. However, in the previous commit 4d34929, we
added `__init__.py` files to the directories under `tests`. This made
it so that the events that we were previously successfully ignoring
had their module updated to their full path, i.e. `tests.unit.<file_name>`,
and thus their module no longer began with `test_`, but `tests.` instead.
Because of this, we had a regression that caused the previously corrected
tests to begin failing again. This commit fixes that.
@QMalcolm QMalcolm requested a review from a team as a code owner May 14, 2024 23:25
@cla-bot cla-bot bot added the cla:yes label May 14, 2024
@codecov-commenter
Copy link

codecov-commenter commented May 14, 2024

Codecov Report

Attention: Patch coverage is 92.30769% with 1 lines in your changes are missing coverage. Please review.

Project coverage is 56.37%. Comparing base (df4b4c0) to head (5ab0f9b).

Files Patch % Lines
dbt_common/events/event_manager.py 87.50% 1 Missing ⚠️
Additional details and impacted files
@@            Coverage Diff             @@
##             main     #132      +/-   ##
==========================================
+ Coverage   56.29%   56.37%   +0.08%     
==========================================
  Files          50       50              
  Lines        3082     3090       +8     
==========================================
+ Hits         1735     1742       +7     
- Misses       1347     1348       +1     
Flag Coverage Δ
unit 56.37% <92.30%> (+0.08%) ⬆️

Flags with carried forward coverage won't be shown. Click here to find out more.

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

@QMalcolm QMalcolm added this pull request to the merge queue May 15, 2024
Merged via the queue into main with commit 3b61b6f May 15, 2024
18 checks passed
@QMalcolm QMalcolm deleted the qmalcolm--131-method-to-add-callbacks-to-event-manager branch May 15, 2024 15:15
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Provide method for adding Callback handlers to global _EVENT_MANAGER
3 participants