Deprecate DatabaseJanitor version argument - #1405
Conversation
📝 WalkthroughWalkthroughThe janitor ChangesJanitor version deprecation
Estimated code review effort: 2 (Simple) | ~10 minutes Possibly related PRs
Suggested reviewers: 🚥 Pre-merge checks | ✅ 6✅ Passed checks (6 passed)
✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@tests/test_janitor.py`:
- Around line 24-27: Replace the Any annotation on version in both deprecation
tests with str | float | Version, and change the parametrized numeric value 10
to 10.0 so it matches the supported input type and BaseDatabaseJanitor.__init__
contract.
- Line 37: Update the version values in the pytest.mark.parametrize decorator to
use a list instead of a tuple, while preserving VERSION, 10, and "10" as the
parameter cases.
🪄 Autofix
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: ASSERTIVE
Plan: Pro Plus
Run ID: 3014c437-3346-468e-b559-cc97cffee6cf
📒 Files selected for processing (9)
README.rstnewsfragments/1393.depr.rstpytest_postgresql/factories/client.pypytest_postgresql/factories/noprocess.pypytest_postgresql/factories/process.pypytest_postgresql/janitor.pytests/test_janitor.pytests/test_noopexecutor.pytests/test_postgres_options_plugin.py
💤 Files with no reviewable changes (6)
- README.rst
- pytest_postgresql/factories/client.py
- tests/test_postgres_options_plugin.py
- pytest_postgresql/factories/noprocess.py
- pytest_postgresql/factories/process.py
- tests/test_noopexecutor.py
Codecov Report✅ All modified and coverable lines are covered by tests. 📢 Thoughts on this report? Let us know! |
Summary
versionargument optional on both janitor classes and emit aDeprecationWarningwhen callers still provide it.Validation
uvx pre-commit run --from-ref upstream/main --to-ref HEADuv run pytest tests/test_janitor.py -q -k 'not test_janitor_populate_async_sql_path and not test_async_janitor_init_and_drop and not test_async_janitor_template_flag_and_context_manager and not test_async_janitor_creates_database_from_template'(31 passed)uv run pytest tests/test_factory_errors.py -q(3 passed)uv buildgit diff --check upstream/main...HEADThe four PostgreSQL-backed janitor tests are deferred to CI because this environment does not provide
pg_configor a PostgreSQL server.Fixes #1393
Summary by CodeRabbit
Deprecation
Documentation
Improvements