chore(api): refresh lock file, constraining the dev toolchain - #8469
chore(api): refresh lock file, constraining the dev toolchain#8469matthewelwell wants to merge 2 commits into
Conversation
|
The latest updates on your projects. Learn more about Vercel for GitHub.
2 Skipped Deployments
|
📝 WalkthroughWalkthroughThe change narrows type-checker suppressions across API permission and service code. It updates DynamoDB query and condition-expression annotations and materialises environment iterables before reuse. It changes optional permission tag annotations to Estimated code review effort: 3 (Moderate) | ~20 minutes Merge Risk: 🔵 Low · up to This dependency refresh and typing cleanup do not show a runtime authorization or DynamoDB behavior regression, but mismatched stub and optional tag-ID contracts can leave the development type-checking environment inconsistent. The impact is bounded to tooling and maintainability. ✨ Finishing Touches 💡 1🛠️ Fix failing CI checks 💡
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 |
github-actions
Bot
added
api
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## main #8469 +/- ##
=======================================
Coverage 98.81% 98.81%
=======================================
Files 1621 1621
Lines 66299 66300 +1
=======================================
+ Hits 65514 65515 +1
Misses 785 785 ☔ View full report in Codecov by Harness. 🚀 New features to boost your workflow:
|
Refreshes api/uv.lock the way Renovate's lock file maintenance would (#8466), with upper bounds added to the dependencies whose upgrades change CI outcomes rather than behaviour. Those upgrades are worth doing, but deliberately and one at a time, not as a side effect of a weekly refresh. Left unconstrained, the refresh produces 935 mypy errors and a test suite that does not collect. Constrained, it produces 38, and every one of them is a real finding rather than churn. 935 unconstrained 731 + dev toolchain constrained (mypy, stubs, pytest-django) 38 + django-lifecycle held below 1.2.5 Constraints: - django-lifecycle <1.2.5. 1.2.5 added a py.typed marker while leaving __init__, save, delete and refresh_from_db unannotated, so mypy stops treating the package as untyped and every call to those methods from typed code becomes a no-untyped-call error: ~490 of them. Held here; the upgrade and its fix are a separate PR. - mypy, djangorestframework-stubs, boto3-stubs pinned to their current minor. These decide what the type checker says about unchanged code. - pytest-django <4.9. 4.14 renames SettingsWrapper to Settings, which breaks the private test suites integrated from flagsmith-private by `make integrate-private-tests` - a repository this one cannot see and whose CI cannot catch the pairing. That rename needs to land there first. - pdbpp widened to >=0.12.1 rather than pinned. It was capped <0.11 while fancycompleter is transitive and unconstrained, so the refresh moved one and not the other; fancycompleter 0.11 removed LazyVersion, which pdbpp 0.10.3 calls at import time, and the entire suite died at collection. pdbpp 0.12.1 requires fancycompleter>=0.11.0, so the pair moves together. The remaining 38 were genuine. Most were stale `# type: ignore` comments that newer stubs made unnecessary. Three were defects the ignores were hiding: - get_permitted_projects, get_permitted_environments and has_environment_permission all declared `tag_ids: typing.List[int] = None` behind an ignore[assignment]; now `list[int] | None`. - get_identity_overrides_key_condition_expression was annotated `-> Key` but returns Key(...).eq(...), a ConditionBase, behind an ignore[return-value]. - _write_environments now materialises its Iterable once. Django's prefetch_related_objects wants a Sequence, and the write loop iterates the same argument again, so a generator would have been silently consumed before anything was written. mypy_boto3_dynamodb also renamed QueryInputRequestTypeDef; this code uses the Table resource API, so the replacement is QueryInputTableQueryTypeDef. Verified: `mypy .` clean across 1882 source files, ruff check and format clean, 107 tests covering the touched permission and model code passing. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
6e42ce7 to
b64de30
Compare
matthewelwell
changed the title
Renovate raises security PRs from manifest entries, so a CVE in a package that
appears only in a lock file is never actioned. That is the bulk of our exposure:
of 95 findings currently open against flagsmith-ee, which shares these four lock
files, 69 are transitive pins and only 26 are declared in a manifest.
There is no longer a dedicated option for this. transitiveRemediation, which
handled exactly this case, was npm@6-only and has been removed from Renovate; it
is absent from the current schema. Refreshing the lock file within existing
ranges is the only remaining mechanism that reaches transitive pins.
security:only-security-updates disables everything via a blanket
matchPackageNames ['*'] rule, so lock file maintenance needs an explicit
packageRule to escape it. The top-level block alone silently no-ops.
Measured against the real backlog before committing:
- frontend/ + docs/ `npm update --package-lock-only` clears 38 of 53 npm
findings. The rest are pinned by a parent or capped by a
declared range, and need an override or a manifest bump.
- mcp/ `uv lock --upgrade` clears both open findings.
- api/ see #8469, which lands the first refresh along with the
constraints that keep it reviewable.
api/ needs no special handling for CodeArtifact. renovate.yml already mints a
token over OIDC and passes it as RENOVATE_HOST_RULES; Renovate's uv processor
reads [[tool.uv.index]] from pyproject.toml, matches the host rule, and exports
UV_INDEX_FLAGSMITH_PYPI_PRODUCTION_{USERNAME,PASSWORD} for the uv subprocess -
precisely what `make codeartifact-login` writes. That path is already exercised
in production, since it is how Renovate resolves flagsmith-private versions for
the eager-bump rule.
Verified with `renovate --platform=local --dry-run=full` on
renovate/renovate:44.62.2-full: four lock file maintenance candidates, none
filtered, grouped into a single renovate/lock-file-maintenance branch. Config
validates against renovate 44.64.0.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Docker builds report
|
❌ private-cloud · depot-ubuntu-latest-16 — run #20193 (attempt 1)Playwright Test Results (private-cloud - depot-ubuntu-latest-16)
Details
Failed testsfirefox › tests/environment-permission-test.pw.ts › Environment Permission Tests › Environment-level permissions control access to features, identities, and segments @enterprise 🗂️ Previous results✅ private-cloud · depot-ubuntu-latest-arm-16 — run #20193 (attempt 1)Playwright Test Results (private-cloud - depot-ubuntu-latest-arm-16)
Details
✅ oss · depot-ubuntu-latest-arm-16 — run #20193 (attempt 1)Playwright Test Results (oss - depot-ubuntu-latest-arm-16)
Details
✅ oss · depot-ubuntu-latest-16 — run #20193 (attempt 1)Playwright Test Results (oss - depot-ubuntu-latest-16)
Details
|
Visual Regression19 screenshots compared. See report for details. |
![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
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: ASSERTIVE
Plan: Team
Run ID: 917dea97-ab27-4f95-831d-7a99ba7fdfc8
⛔ Files ignored due to path filters (1)
api/uv.lockis excluded by!**/*.lock
📒 Files selected for processing (16)
api/audit/views.pyapi/core/redis_cluster.pyapi/core/types.pyapi/environments/dynamodb/wrappers/environment_wrapper.pyapi/environments/dynamodb/wrappers/identity_wrapper.pyapi/experimentation/ingestion_sync_service.pyapi/features/feature_states/permissions.pyapi/features/import_export/permissions.pyapi/features/permissions.pyapi/features/versioning/permissions.pyapi/organisations/permissions/permissions.pyapi/projects/code_references/permissions.pyapi/pyproject.tomlapi/tests/unit/features/test_unit_features_views.pyapi/users/models.pydocs/docs/deployment-self-hosting/observability/_events-catalogue.md
Included review availability: Your plan provides up to 8 included reviews per hour; 6 remain after this review.
| ) | ||
| # ... and then build and return the client | ||
| return RedisCluster(**client_cls_kwargs) # type: ignore[abstract] | ||
| return RedisCluster(**client_cls_kwargs) # type: ignore[no-any-return] |
There was a problem hiding this comment.
📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick win
🔎 Supported by static analysis
🤖 get_repo_knowledge executed:
get_repo_knowledge Flagsmith/flagsmith /tmp/coderabbit-repo-knowledge/flagsmith-flagsmith-a74d157a/learnings /tmp/coderabbit-repo-knowledge/flagsmith-flagsmith-a74d157a/architecture
Length of output: 40238
🏁 Script executed:
#!/bin/bash
set -eu
for spec in \
"api/core/redis_cluster.py:120:138" \
"api/experimentation/ingestion_sync_service.py:1:30" \
"api/features/feature_states/permissions.py:1:25" \
"api/features/import_export/permissions.py:1:70" \
"api/organisations/permissions/permissions.py:220:240"
do
file=${spec%%:*}
rest=${spec#*:}
start=${rest%%:*}
end=${rest##*:}
printf '\n--- %s:%s-%s ---\n' "$file" "$start" "$end"
sed -n "${start},${end}p" "$file" | nl -ba -v "$start"
done
printf '\n--- relevant typing configuration and local ignore guidance ---\n'
rg -n --glob '!*lock*' --glob '!*.min.*' \
'type: ignore|disallow_any|warn_return_any|mypy|pyright|typing' \
pyproject.toml setup.cfg tox.ini .pre-commit-config.yaml .github 2>/dev/null | head -200Repository: Flagsmith/flagsmith
Length of output: 241
🏁 Script executed:
#!/bin/bash
set -eu
for spec in \
"api/core/redis_cluster.py:120:138" \
"api/experimentation/ingestion_sync_service.py:1:30" \
"api/features/feature_states/permissions.py:1:25" \
"api/features/import_export/permissions.py:1:70" \
"api/organisations/permissions/permissions.py:220:240"
do
file=${spec%%:*}
rest=${spec#*:}
start=${rest%%:*}
end=${rest##*:}
printf '\n--- %s:%s-%s ---\n' "$file" "$start" "$end"
sed -n "${start},${end}p" "$file"
done
printf '\n--- relevant typing configuration and local ignore guidance ---\n'
rg -n --glob '!*lock*' --glob '!*.min.*' \
'type: ignore|disallow_any|warn_return_any|mypy|pyright|typing' \
pyproject.toml setup.cfg tox.ini .pre-commit-config.yaml .github 2>/dev/null | head -200Repository: Flagsmith/flagsmith
Length of output: 6824
🤖 get_repo_knowledge executed:
get_repo_knowledge Flagsmith/flagsmith /tmp/coderabbit-repo-knowledge/flagsmith-flagsmith-a74d157a
Length of output: 860
Document the retained # type: ignore[...] directives.
Add a short reason or removal condition to each listed suppression. The no-any-return directives suppress Redis client return typing, and the union-attr directives suppress permission-call typing on request.user. The surrounding code does not document these typing limitations.
📍 Affects 5 files
api/core/redis_cluster.py#L133-L133(this comment)api/experimentation/ingestion_sync_service.py#L21-L21api/features/feature_states/permissions.py#L17-L17api/features/import_export/permissions.py#L22-L22api/features/import_export/permissions.py#L31-L31api/features/import_export/permissions.py#L41-L41api/features/import_export/permissions.py#L52-L52api/features/import_export/permissions.py#L63-L63api/organisations/permissions/permissions.py#L233-L233
Source: Learnings
| "mypy-boto3-dynamodb>=1.33.0,<2.0.0", | ||
| "mypy-boto3-s3>=1.36.0,<2.0.0", |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win
🔎 Supported by static analysis
🏁 Script executed:
#!/usr/bin/env bash
set -euo pipefail
(
cd api
uv lock --check
)
rg -n -A2 'name = "(boto3|boto3-stubs|mypy-boto3-dynamodb|mypy-boto3-s3)"' api/uv.lockRepository: Flagsmith/flagsmith
Length of output: 197
Align the Boto3 stub version bounds.
boto3-stubs is limited to <1.37.0, while mypy-boto3-dynamodb and mypy-boto3-s3 allow versions below 2.0.0. A lock refresh may select incompatible stub versions and reintroduce type-checking drift. Apply the same version window to all three packages, or pin compatible versions in api/uv.lock.
| tag_ids: list[int] | None = None, | ||
| ) -> QuerySet[Project]: | ||
| return get_permitted_projects_for_user(self, permission_key, tag_ids) | ||
| return get_permitted_projects_for_user(self, permission_key, tag_ids) # type: ignore[arg-type] |
There was a problem hiding this comment.
📐 Maintainability & Code Quality | 🟠 Major | ⚡ Quick win
🔎 Supported by static analysis
🏁 Script executed:
rg -n -A10 -B2 'def get_permitted_(projects|environments)_for_user' api/permissions/permission_service.pyRepository: Flagsmith/flagsmith
Length of output: 1028
🏁 Script executed:
#!/bin/bash
set -e
printf '%s\n' '--- api/users/models.py ---'
sed -n '320,395p' api/users/models.py
printf '%s\n' '--- api/permissions/permission_service.py ---'
sed -n '1,175p' api/permissions/permission_service.py
printf '%s\n' '--- repository type/version context ---'
rg -n 'python_requires|target-version|requires-python|python_version|from __future__ import annotations' pyproject.toml setup.cfg setup.py tox.ini .pre-commit-config.yaml api/users/models.py api/permissions/permission_service.py 2>/dev/null || trueRepository: Flagsmith/flagsmith
Length of output: 9748
🤖 get_repo_knowledge executed:
get_repo_knowledge Flagsmith/flagsmith /tmp/coderabbit-repo-knowledge/flagsmith-flagsmith-a74d157a/architecture /tmp/coderabbit-repo-knowledge/flagsmith-flagsmith-a74d157a/learnings /tmp/coderabbit-repo-knowledge/flagsmith-flagsmith-a74d157a/conventions
Length of output: 46814
Align the optional tag_ids contract across the permission layer.
get_permitted_projects_for_user and get_permitted_environments_for_user accept None at runtime but declare List[int]. Change these parameters and has_environment_permission.tag_ids to list[int] | None, then remove the matching type suppressions.
1 participant
Refreshes
api/uv.lockthe way Renovate's lock file maintenance would (#8466), with a couple of caveats to restrict the scope of this PR:django-lifecyclewhich results in a large number of typing fixes. This will be done in it's own PR.