Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Added a DynamoDb backend #334

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

ajenkinski
Copy link

@ajenkinski ajenkinski commented Apr 4, 2022

Adds an AWS DynamoDB cache backend to contrib. The DynamoDbCache class has documentation on usage.

Checklist:

  • Add tests that demonstrate the correct behavior of the change. Not applicable. I saw a rejected PR for another backend where you said you didn't want tests for contributed backends. I can add tests if you like.
  • Add or update relevant docs, in the docs folder and in code.
  • Add an entry in CHANGES.rst summarizing the change and linking to the issue.
  • Add .. versionchanged:: entries in any relevant code docs.
  • Run pre-commit hooks and fix any issues. All the pre-commit steps complete except the "black" stage. See below for details.
  • Run pytest and tox, no tests failed.

When the pre-commit hook runs, this is the output I get:

pyupgrade................................................................Passed
Reorder python imports...................................................Passed
black....................................................................Failed
- hook id: black
- exit code: 1

Traceback (most recent call last):
  File "/root/.cache/pre-commit/repotslw8qcx/py_env-python3.7/bin/black", line 8, in <module>
    sys.exit(patched_main())
  File "/root/.cache/pre-commit/repotslw8qcx/py_env-python3.7/lib/python3.7/site-packages/black/__init__.py", line 1282, in patched_main
    patch_click()
  File "/root/.cache/pre-commit/repotslw8qcx/py_env-python3.7/lib/python3.7/site-packages/black/__init__.py", line 1268, in patch_click
    from click import _unicodefun  # type: ignore
ImportError: cannot import name '_unicodefun' from 'click' (/root/.cache/pre-commit/repotslw8qcx/py_env-python3.7/lib/python3.7/site-packages/click/__init__.py)

flake8...................................................................Passed
fix UTF-8 byte order marker..............................................Passed
Trim Trailing Whitespace.................................................Passed
Fix End of Files.........................................................Passed

It doesn't seem to have anything to do with my code, and I couldn't figure out what's installing stuff in that ~/.cache directory. It seems that the code in black/__init__.py expects click to have a _unicodefun symbol, which isn't present in the version of click that's getting installed in that .cache directory. Since I didn't change anything in the requirements files, this doesn't seem related to my changes.

@northernSage
Copy link
Member

Hey @ajenkinski, thanks for the contribution! This look good to me, but with #308 we are in the process of moving all backend cache classes to cachelib so this should probably be moved there. The pre-commit problem seems to have been fixed with #335 (new black release), so it should be fine now.

@ajenkinski
Copy link
Author

Would you like me to move it to cachelib, or will you? If I do it, where should it go? I don't see a contrib package in cachelib.

@northernSage
Copy link
Member

@ajenkinski That would be very helpful if you find the time. All cache classes in cachelib have their own separate modules which are all located in https://github.com/pallets-eco/cachelib/tree/main/src/cachelib. You would have to create a new one for your new DynamoDbCache (something like dynamo.py) and add to https://github.com/pallets-eco/cachelib/blob/main/src/cachelib/__init__.py like the other ones. Feel free to ping ('@') me if you have any questions. Thanks!

@northernSage northernSage self-assigned this May 27, 2022
@wangsha wangsha mentioned this pull request Jan 6, 2023
5 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Development

Successfully merging this pull request may close these issues.

Support DynamoDB as a backend
2 participants