fix: correct type annotations and improve CI cache invalidation#579
Conversation
|
The PR description has been updated. Please fill out the template for your PR to be reviewed. |
Merge ProtectionsYour pull request matches the following merge protections and will not be merged until they are valid. 🟢 Enforce conventional commitWonderful, this rule succeeded.Make sure that we follow https://www.conventionalcommits.org/en/v1.0.0/
|
- Fix incorrect **kwargs type annotation in elasticsearch.py (dict[str, int] -> Any) - Remove unnecessary type: ignore comments for imports now in pyproject.toml - Update CI pre-commit cache key to include uv.lock hash - Ensures MyPy re-checks files when dependencies change Fixes type errors introduced in generative-computing#571 that were masked by stale CI cache. Signed-off-by: Alex Bozarth <ajbozart@us.ibm.com>
cec5261 to
45031b1
Compare
|
thanks for the quick investigation.
Is this a bug in our ci/cd or does this fall into the category of stuff we'd rather handle with nightlies so that the github actions ci/cd has fast-as-possible turn-around? |
I would consider it a bug, but it's up to interpretation. I think the current cache hashing was overzealous and had the potential to hid issues caused by dependency version updates. Though this fix can end up slowing down the CI it is a drop in the bucket compared to out tests |
|
I think some slow-down is fine - mypy breakage is deeply disruptive to all downstream work. IMO the balance of concerns here justified invalidation. |
…rative-computing#579) - Fix incorrect **kwargs type annotation in elasticsearch.py (dict[str, int] -> Any) - Remove unnecessary type: ignore comments for imports now in pyproject.toml - Update CI pre-commit cache key to include uv.lock hash - Ensures MyPy re-checks files when dependencies change Fixes type errors introduced in generative-computing#571 that were masked by stale CI cache. Signed-off-by: Alex Bozarth <ajbozart@us.ibm.com>
Misc PR
Type of PR
Description
Fix MyPy Type Errors and CI Cache Invalidation
Problem
MyPy type errors were occurring locally but not caught in CI:
Root Cause
**kwargsannotation (dict[str, int]instead ofAny) inelasticsearch.pyuv.lock, so MyPy used stale cached results when dependencies changedThe bug was introduced when PR #571 (code) and PR #570 (dependencies) were merged separately. CI never re-checked the files with the new type information because the cache wasn't invalidated.
Changes
Type Fix
**kwargs: dict[str, int]→**kwargs: Anyinelasticsearch.pyCI Fix
uv.lockhashTesting
✅
pre-commit run --all-filespasses locally✅ All MyPy type checks pass
Impact
Prevents future type errors from being masked by stale CI cache when dependencies are added or updated.
Testing