requests-cache / aiohttp-client-cache

An async persistent cache for aiohttp requests
MIT License
116 stars 20 forks source link

Fix/update type annotations #225

Closed alessio-locatelli closed 6 months ago

alessio-locatelli commented 6 months ago

There were some tricky places where I had two choices: a major refactoring or a "best effort" attempt to fix the type annotations.

A simplified example to explain a challenge I had to solve in the "test/" folder:

class BaseStorageTest:
    storage_class: type[BaseCache]  # This is tricky because we assign child classes that have more different attributes.

   def init_cache() -> BaseCache:  # This is tricky because this is a base class, but we always return a child class (Mongo, Redis, SQLite, etc.)
       yield self.storage_class

class TestMongoDBCache(BaseStorageTest):
    storage_class: MongoDBCache

    # Here we have many warnings about `BaseCache` has no attribute [...] because `init_cache` says it returns `BaseCache`.
    # For example, `BaseCache` has not attribute named `collection`.

class TestSQLiteCache(BaseStorageTest):
    storage_class: SQLiteCache

    # Here we have many warnings about `BaseCache` has no attribute [...] because `init_cache` says it returns `BaseCache`.
    # Type checkers do not know abut `SQLiteCache` *unique* attributes, so we have no IDE autocompletion and warnings from linters.

I was thinking about how to get IDE autocompletion and fix Mypy/Pyright warnings and come up with as small refactorings as possible, but better ideas are welcome.
The workaround was to define that if we pass type[SQLiteCache] then we get SQLiteCache, etc.

Another tricky task was the fact that you return -> CachedResponse | ClientResponse but in the tests you always check attributes of CachedResponse (e.g. from_cache). The simplest workaround was using cast() to tell IDE and a type checker that we work in the test with the CachedResponse.


CI for 3.11 and 3.12 failed with

cache/.venv/bin/python: bad interpreter: No such file or directory
Error: Process completed with exit code 126.

:heavy_check_mark: Locally I successfully executed all tests for 3.8 and 3.12.

codecov[bot] commented 6 months ago

Codecov Report

Attention: Patch coverage is 97.95918% with 1 lines in your changes are missing coverage. Please review.

Project coverage is 97.75%. Comparing base (3aa2e5e) to head (5b54454).

Files Patch % Lines
aiohttp_client_cache/response.py 92.85% 1 Missing :warning:
Additional details and impacted files ```diff @@ Coverage Diff @@ ## main #225 +/- ## ========================================== + Coverage 97.22% 97.75% +0.52% ========================================== Files 10 10 Lines 1010 1025 +15 Branches 171 173 +2 ========================================== + Hits 982 1002 +20 + Misses 18 16 -2 + Partials 10 7 -3 ```

:umbrella: View full report in Codecov by Sentry.
:loudspeaker: Have feedback on the report? Share it here.