-
Notifications
You must be signed in to change notification settings - Fork 0
QDB-16178 - Refactor python statistics module #93
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
QDB-16178 - Refactor python statistics module #93
Conversation
…6178/refactor-python-statistics-module-to-make-use
Reuses venv, reuses `build/` directory
…ython-api' into sc-16178/refactor-python-statistics-module-to-make-use
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.
Pull Request Overview
This pull request refactors the Python statistics module and its tests. Key changes include updating the logic in statistics retrieval functions in quasardb/stats.py, refactoring the polling mechanism in tests/test_stats.py, and deprecating the stat_type API with a warning notice.
Reviewed Changes
Copilot reviewed 2 out of 3 changed files in this pull request and generated 1 comment.
| File | Description |
|---|---|
| tests/test_stats.py | Updates to the _ensure_stats polling logic and test setup |
| quasardb/stats.py | Refactoring of key retrieval, indexing, and metric type/unit lookup |
Files not reviewed (1)
- scripts/tests/setup: Language not supported
Comments suppressed due to low confidence (1)
quasardb/stats.py:101
- The 'stat_type' function now only issues a deprecation warning and does not return any value, which could break existing usage that expects a stat type. Consider returning the legacy value or updating consumers to use the new API.
def stat_type(stat_id):
igorniebylski
left a comment
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.
LGTM
rodp63
left a comment
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.
2 minor comments, LGTM
Makes use of the new server-side metric type/units, and changes the way statistics are exposed. This is a breaking change for users who previously used the statistics API.
No description provided.