Refactors Redis Persister

[skrawcz]

This adds a new class RedisBasePersister that the old one inherits from.

The reason to do this is that if someone wants to test things, then being able to inject a mock redis client makes this simpler. So refactoring to enable that while still being backwards compatible.

Appropriately marked things as deprecated and tests have been updated.

Changes

• refactors redis persister

How I tested this

• unit tests

Notes

• request by user to make testing simpler.

Checklist

• PR has an informative and human-readable title (this will be pulled into the release notes)
• Changes are limited to a single goal (no scope creep)
• Code passed the pre-commit check & code is left cleaner/nicer than when first encountered.
• Any change in functionality is tested
• New functions are documented (with a description, list of inputs, and expected output)
• Placeholder code is flagged / future TODOs are captured in comments
• Project documentation has been updated if adding/changing functionality.

---

Important

Refactors Redis persister by introducing RedisBasePersister for easier testing, deprecating RedisPersister, and updating tests and documentation.

• Refactoring:
  • Introduces RedisBasePersister class in b_redis.py for easier testing with mock Redis clients.
  • RedisPersister now inherits from RedisBasePersister and is marked as deprecated.
• Backward Compatibility:
  • RedisPersister class remains functional for backward compatibility.
• Documentation:
  • Updates persister.rst to reference RedisBasePersister instead of RedisPersister.
• Testing:
  • Updates tests in test_b_redis.py to use RedisBasePersister.
  • Adds test to ensure RedisPersister remains backward compatible.

^{This description was created by for 428224d. It will automatically update as commits are pushed.}

[github-actions]

A preview of 428224d is uploaded and can be seen here:

✨ https://burr.dagworks.io/pull/471 ✨

Changes may take a few minutes to propagate. Since this is a preview of production, content with draft: true will not be rendered. The source is here: https://github.com/DAGWorks-Inc/burr/tree/gh-pages/pull/471/

[ellipsis-dev]

👍 Looks good to me! Reviewed everything up to 0df8999 in 1 minute and 16 seconds

More details

• Looked at 176 lines of code in 3 files
• Skipped 0 files when reviewing.
• Skipped posting 2 drafted comments based on config settings.

1. burr/integrations/persisters/b_redis.py:31

• Draft comment:
  The from_values method should not be a class method since it does not use the cls parameter to create an instance. Consider removing the @classmethod decorator.
• Reason this comment was not posted:
  Comment looked like it was already resolved.

2. burr/integrations/persisters/b_redis.py:161

• Draft comment:
  Using __del__ for closing connections is not reliable. Consider using a context manager to ensure the connection is closed properly.
• Reason this comment was not posted:
  Comment was on unchanged code.

Workflow ID: wflow_d6od0ep7AYj0OSCD

---

You can customize Ellipsis with 👍 / 👎 feedback, review rules, user-specific overrides, quiet mode, and more.

[ellipsis-dev]

👍 Looks good to me! Incremental review on 428224d in 14 seconds

More details

• Looked at 9 lines of code in 1 files
• Skipped 0 files when reviewing.
• Skipped posting 1 drafted comments based on config settings.

1. tests/integrations/persisters/test_b_redis.py:72

• Draft comment:
  The import of RedisPersister is unnecessary as it is not used in the tests. Consider removing it to clean up the code.
• Reason this comment was not posted:
  Confidence changes required: 50%
  The test test_redis_persister_class_backwards_compatible correctly tests the backward compatibility of the RedisPersister class. However, the import of RedisPersister is unnecessary in the context of the other tests, which only use RedisBasePersister. This import should be removed to clean up the code.

Workflow ID: wflow_nAKrmiFjVykMFLSI

---

You can customize Ellipsis with 👍 / 👎 feedback, review rules, user-specific overrides, quiet mode, and more.