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

Brings back EventPersister in discovery of background services #17240

Merged
merged 1 commit into from
Feb 21, 2025

Conversation

chrisguidry
Copy link
Collaborator

In the prior work on this (#16913) we needed to
maintain an explicit set of modules from which to discover services, and
I missed the modules that include the EventPersister and the
EventLogger (a debugging-only service). This meant that under most
circumstances, because the event_persister module wasn't imported, we
wouldn't run it in either API or background processes.

Here I'm adding a test that confirms the exact subsets of services we
should run, just to avoid any quiet failures like this.

Checklist

  • This pull request references any related issue by including "closes <link to issue>"
    • If no issue exists and your change is not a small fix, please create an issue first.
  • If this pull request adds new functionality, it includes unit tests that cover the changes
  • If this pull request removes docs files, it includes redirect settings in mint.json.
  • If this pull request adds functions or classes, it includes helpful docstrings.

In the prior work on this (#16913) we needed to
maintain an explicit set of modules from which to discover services, and
I missed the modules that include the `EventPersister` and the
`EventLogger` (a debugging-only service).  This meant that under most
circumstances, because the `event_persister` module wasn't imported, we
wouldn't run it in either API or background processes.

Here I'm adding a test that confirms the exact subsets of services we
should run, just to avoid any quiet failures like this.
@cicdw cicdw added the bug Something isn't working label Feb 21, 2025
Copy link

codspeed-hq bot commented Feb 21, 2025

CodSpeed Performance Report

Merging #17240 will not alter performance

Comparing include-event-persister (f885834) with main (a7e94ad)

Summary

✅ 2 untouched benchmarks

@zzstoatzz zzstoatzz merged commit d4d9001 into main Feb 21, 2025
52 checks passed
@zzstoatzz zzstoatzz deleted the include-event-persister branch February 21, 2025 19:35
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants