Skip to content

Conversation

@kossman
Copy link
Contributor

@kossman kossman commented Sep 15, 2025

Changes

  • Added separate migrator class HashMigrator to encap hash redis keys migration in BC way (by creating new key with separate prefix to not break old keys if they will be needed while reverting)
  • upgrade redis version for test server
  • added "hack" for FakeRedis class since by default it doesn't support .info() method
  • bump bunch of related third party dependencies:
    • fakeredis[lua] -> removed prefix LUA
    • redis == 6.2.0 latest supported with Python3.9+
    • types-redis~=4.6.0
  • removed:
    • all LUA bindings from RedisRepository
    • use_lua_52
    • vacuum(...) and dependant codes
    • InternalCacheSdkProtocol
    • InternalRedisSdk
    • FakeInternalCacheSdk
    • DeprecatedRedisAdapter

DEVC-1286

@kossman kossman marked this pull request as ready for review September 18, 2025 11:48
@kossman kossman changed the title feat(DEVC-1286): init changes feat(DEVC-1286): Revamp redis usage Sep 18, 2025
returns: stored data
def check_redis_server_version(self) -> None:
# Require Redis 7.4+ for per-field TTL commands
redis_version_str = self.client.info(section="server")["redis_version"]
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

just curious, does this self.client.info() actually queries redis server or is it generated once after connection is established? Because if it queries stuff than you should probably do this once in init, store it in some attribute and then use attribute here

redis_client = redis.Redis.from_url(url=redis_dsn, decode_responses=True)

migrator = HashMigrator(hash_name, redis_client)
migrator.run()
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

so you want to run this on every execution even if app will not touch cache at all? It's probably a micro-optimization but a lot of apps don't actually use cache at all, so maybe let's do it in "lazy" way - trigger this .run() when app tries to read/write/delete something for the first time? Add some flag attribute to this UserRedisSdk class, set it to False by default. If user calls one of the methods (get, set, ...) you can check if attribute is False, if it is - call that .run() method and set flag to true.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Sounds great, like it, will add this soon, thanks! 🚀

@kossman kossman merged commit ea17c1a into feature/DEVC-1282-migrate-to-pydantic-v2 Sep 24, 2025
14 checks passed
@kossman kossman deleted the feature/DEVC-1286-revamp-redis-usage branch September 24, 2025 11:23
kossman added a commit that referenced this pull request Sep 24, 2025
* feat: remove redis ping
* feat(DEVC-265): add py-typed file & update pyproject.toml accordingly
* feat(DEVC-265): ensure that lint passed; bump mypy version from mypy==0.950 -> mypy>=1.10,<2
* feat(DEVC-265): move mypy CLI args into single place - pyproject.toml file
* feat(DEVC-265): remove exclude block at mypy settings for skipping some docs/modules tutorials
* feat(DEVC-265): optimise mypy config
* feat(DEVC-1282): migrate SDK code to pydantic V2
* feat(DEVC-1282): migrate tests to pydantic v2 as well; add some fixes
* feat(DEVC-1282): improve type annotations; refactor RerunTimeRange class validators at to simplify
* feat(DEVC-1282): fix annotations for `model_validator` with mode="before"
* feat(DEVC-1282): set new minor version
* feat(DEVC-1286): Revamp redis usage (#111)

- upgrade redis version for test server
- bump bunch of related third party dependencies:
  - fakeredis[lua] -> removed prefix LUA
  - redis == 6.2.0 latest supported with Python3.9+
  - types-redis~=4.6.0
- removed:
  - all LUA bindings from RedisRepository
  - use_lua_52
  - vacuum(...) and dependant codes
  - InternalCacheSdkProtocol
  - InternalRedisSdk
  - FakeInternalCacheSdk
  - DeprecatedRedisAdapter
- Add migration logic
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants