-
Notifications
You must be signed in to change notification settings - Fork 1.5k
Revert "Migrate MySQL to new schema collector" #22103
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
Revert "Migrate MySQL to new schema collector" #22103
Conversation
This reverts commit 569fb2a.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
💡 Codex Review
Here are some automated review suggestions for this pull request.
ℹ️ About Codex in GitHub
Codex has been enabled to automatically review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".
| mock.patch('datadog_checks.mysql.databases_data.DatabasesData._get_tables', return_value=[1, 2]), | ||
| ): | ||
| with pytest.raises(StopIteration): | ||
| databases_data._fetch_database_data("dummy_cursor", time.time(), "my_db") |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
test_fetch_throws fails before expected StopIteration
When time.time is patched to return a huge elapsed duration, _fetch_database_data immediately calls send_truncated_msg, which indexes self._data_submitter.db_info[db_name]. In this test no call to store_db_infos populates that mapping, so the call raises a KeyError before the StopIteration the test asserts, causing the new unit test to fail deterministically. Seed the db metadata (e.g., via store_db_infos([{"name": "my_db"}])) or adjust the assertion to match the actual exception.
Useful? React with 👍 / 👎.
Review from sethsamuel is dismissed. Related teams and files:
- database-monitoring
- mysql/CHANGELOG.md
- database-monitoring-agent
- mysql/changelog.d/22103.fixed
Codecov Report❌ Patch coverage is Additional details and impacted files🚀 New features to boost your workflow:
|
This reverts commit 8b4e2b7.
* Revert "Migrate MySQL to new schema collector (#21729)" This reverts commit 569fb2a. * Add changelog (cherry picked from commit 8b4e2b7) Co-authored-by: Eric Weaver <[email protected]>
Reverts #21729
We need to revert this schem collection refactor. These optimized schema collection queries break schema collection on many older Mysql / MariaDB versions due to introduction of newer functions. Pulling this out of 7.74 release in order to gracefully support falling back to this existing collection method on unsupportable versions in a future agent release