uv upgrade#169
Merged
Merged
Conversation
- Replace requirements.in / requirements-dev.in / requirements.txt / requirements-dev.txt with pyproject.toml config + uv.lock - Move production dependencies to [project].dependencies - Move dev dependencies to [dependency-groups].dev - Move uwsgi and uwsgitop to [dependency-groups].prod - Add [tool.uv] configuration with package = false - Add [tool.uv] exclude-newer = "P3D" (supply chain protection) - Update README-instructions for uv - Add .venv to .gitignore Refs: NS-209
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Run ID: ⛔ Files ignored due to path filters (1)
📒 Files selected for processing (16)
💤 Files with no reviewable changes (1)
✅ Files skipped from review due to trivial changes (3)
🚧 Files skipped from review as they are similar to previous changes (9)
📝 WalkthroughWalkthroughThe PR migrates Python dependency management from Changespip → uv dependency management migration
DatabaseHealthCheck framework refactor
ResilientLogSourceEntry test fixture and import updates
Sequence Diagram(s)sequenceDiagram
participant Client
participant HealthCheckJSONView
participant Executor
participant DatabaseHealthCheck
Client->>HealthCheckJSONView: GET /healthz/
HealthCheckJSONView->>Executor: asyncio.gather(configured checks)
Executor->>DatabaseHealthCheck: run()
DatabaseHealthCheck-->>Executor: result or ServiceUnavailable
Executor-->>HealthCheckJSONView: results[]
HealthCheckJSONView->>HealthCheckJSONView: derive status from result.error
HealthCheckJSONView-->>Client: JSON response with 200 or 500
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Poem
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches📝 Generate docstrings
🧪 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 |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 3
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (2)
custom_health_checks/backends.py (2)
21-22: 🔒 Security & Privacy | 🟠 Major | ⚡ Quick winAvoid exposing raw database errors in health responses.
At Lines 21-22, interpolating
eintoServiceUnavailablecan leak internal DB details through/healthz. Return a generic message and chain the original exception instead. This will also require updating the downstream assertion incustom_health_checks/tests/test_healthz.pyLine 29.Suggested fix
-from django.db import connection +from django.db import DatabaseError, connection @@ - except Exception as e: - raise ServiceUnavailable(f"Database connection failed: {e}") + except DatabaseError as e: + raise ServiceUnavailable("Database connection failed") from e🤖 Prompt for 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. In `@custom_health_checks/backends.py` around lines 21 - 22, The ServiceUnavailable exception in the except block is exposing raw database error details through the health check endpoint, which is a security concern. Replace the f-string that interpolates the error `e` with a generic message like "Database connection failed" and chain the original exception using the `from e` syntax to preserve the error context for logging without exposing it in the response. Additionally, update the assertion in the test file `custom_health_checks/tests/test_healthz.py` at Line 29 to expect the generic message instead of the actual database error details.
20-20: 🎯 Functional Correctness | 🟠 Major | ⚡ Quick winUse a table-agnostic DB probe query.
At Line 20, coupling health status to
users_usercan produce false negatives (e.g., custom user table, migration timing) even when DB connectivity is healthy.Suggested fix
- cursor.execute("SELECT 1 FROM users_user LIMIT 1") + cursor.execute("SELECT 1")🤖 Prompt for 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. In `@custom_health_checks/backends.py` at line 20, The cursor.execute call targeting the users_user table creates unnecessary coupling to a specific table, causing false negatives when that table doesn't exist or during migrations even if database connectivity is healthy. Replace the query with a table-agnostic health check that tests database connectivity without referencing any specific table, such as a simple constant query like SELECT 1.
🤖 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 @.github/workflows/ci.yml:
- Line 13: The workflow file uses unconditional `secrets: inherit` which
forwards all available secrets to the called workflow without explicit control.
Replace the `secrets: inherit` line with explicit secret mapping by specifying
only the required secrets. List each secret that the reusable workflow needs
under a `secrets:` field with explicit mappings like `SECRET_NAME: ${{
secrets.SECRET_NAME }}` for each secret required, following the principle of
least privilege.
- Line 12: The reusable workflow reference in the uses statement is pinned to
the `@main` branch reference, which is mutable and poses a supply chain security
risk. Replace the `@main` reference with a full commit SHA from the
City-of-Helsinki/.github repository to ensure immutable and deterministic
workflow behavior. Identify a stable commit SHA from that repository and update
the uses statement to point to that specific commit instead of the branch.
In `@pyproject.toml`:
- Around line 16-17: The `factory-boy` and `ipython` packages are currently in
the `[project].dependencies` section of pyproject.toml, which causes them to be
installed in production environments. Since `factory-boy` is only used by test
factory files and `ipython` is unused, these packages should be moved to a test
or development dependency group. Remove `factory-boy` and `ipython` from
`[project].dependencies` and add them to an appropriate optional dependencies
group in pyproject.toml (such as under `[project.optional-dependencies]` with a
`dev` or `test` key) so they are only installed during development.
---
Outside diff comments:
In `@custom_health_checks/backends.py`:
- Around line 21-22: The ServiceUnavailable exception in the except block is
exposing raw database error details through the health check endpoint, which is
a security concern. Replace the f-string that interpolates the error `e` with a
generic message like "Database connection failed" and chain the original
exception using the `from e` syntax to preserve the error context for logging
without exposing it in the response. Additionally, update the assertion in the
test file `custom_health_checks/tests/test_healthz.py` at Line 29 to expect the
generic message instead of the actual database error details.
- Line 20: The cursor.execute call targeting the users_user table creates
unnecessary coupling to a specific table, causing false negatives when that
table doesn't exist or during migrations even if database connectivity is
healthy. Replace the query with a table-agnostic health check that tests
database connectivity without referencing any specific table, such as a simple
constant query like SELECT 1.
🪄 Autofix (Beta)
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: defaults
Review profile: CHILL
Plan: Pro
Run ID: 650e894d-8a28-42ad-9bd0-650ff836f2a2
⛔ Files ignored due to path filters (1)
uv.lockis excluded by!**/*.lock
📒 Files selected for processing (22)
.dockerignore.github/dependabot.yml.github/workflows/ci.yml.gitignore.pre-commit-config.yamlDockerfileREADME.mdapi/tests/conftest.pyapi/tests/test_models.pyaudit_log/tests/conftest.pyaudit_log/tests/test_services.pycustom_health_checks/apps.pycustom_health_checks/backends.pycustom_health_checks/tests/test_backends.pycustom_health_checks/tests/test_healthz.pycustom_health_checks/views.pypyproject.tomlrelease-please-config.jsonrequirements-dev.inrequirements-dev.txtrequirements.inrequirements.txt
💤 Files with no reviewable changes (5)
- requirements.in
- requirements-dev.in
- requirements-dev.txt
- custom_health_checks/apps.py
- requirements.txt
|
NOTIFICATION-SERVICE-API branch is deployed to platta: https://notification-service-api-pr169.dev.hel.ninja 🚀🚀🚀 |
mhieta
force-pushed
the
NS-209/uv-upgrade
branch
from
44c7d74 to
26f7ad9
Compare
|
NOTIFICATION-SERVICE-API branch is deployed to platta: https://notification-service-api-pr169.dev.hel.ninja 🚀🚀🚀 |
Also add .dockerignore-file. Refs: NS-209
Refs: NS-209
Refs: NS-209
Refs: NS-209
Replace ResilientLogSource(entry) instantiation with ResilientLogSourceEntry(entry) in tests. Add RESILIENT_LOGGER environment override and cache clearing to test conftests to satisfy non-empty environment validation. Refs: NS-209
Replace BaseHealthCheckBackend/plugin_dir/MainView with HealthCheck/ HealthCheckView. Switch check_status() to run() and register checks via HealthCheckView.checks instead of plugin_dir. Refs: NS-209
Refs: NS-209
mhieta
force-pushed
the
NS-209/uv-upgrade
branch
from
26f7ad9 to
1601bc4
Compare
|
|
NOTIFICATION-SERVICE-API branch is deployed to platta: https://notification-service-api-pr169.dev.hel.ninja 🚀🚀🚀 |
voneiden
approved these changes
Jun 25, 2026
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Refs: NS-209
Summary by CodeRabbit