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

Feat: support user storage proxy for string type doc_id #224

Merged
merged 6 commits into from Aug 9, 2018

Conversation

@Cologler
Copy link
Contributor

Cologler commented Aug 4, 2018

for #221 .

examples see the added unittest.

@msiemens msiemens self-requested a review Aug 9, 2018
Copy link
Owner

msiemens left a comment

One change request, otherwise this should be fine 🙂

@@ -77,6 +77,10 @@ def __init__(self, storage, table_name):
self._storage = storage
self._table_name = table_name

def construct_doc(self, key, val):

This comment has been minimized.

Copy link
@msiemens

msiemens Aug 9, 2018

Owner

Could we rename this to an internal method and also use something like _new_document(…)?

This comment has been minimized.

Copy link
@Cologler

Cologler Aug 9, 2018

Author Contributor

a new commit for that.

This comment has been minimized.

Copy link
@msiemens

msiemens Aug 9, 2018

Owner

Awesome! Now you just need to adapt the example in

tinydb/tests/test_tinydb.py

Lines 617 to 618 in a454998

def construct_doc(self, key, val):
return Document(val, key)
🙂

This comment has been minimized.

Copy link
@Cologler

Cologler Aug 9, 2018

Author Contributor

😂

@msiemens msiemens added the enhancement label Aug 9, 2018
@msiemens msiemens merged commit dbd449d into msiemens:master Aug 9, 2018
2 checks passed
2 checks passed
continuous-integration/travis-ci/pr The Travis CI build passed
Details
coverage/coveralls Coverage remained the same at 100.0%
Details
@msiemens

This comment has been minimized.

Copy link
Owner

msiemens commented Aug 9, 2018

Thanks for the idea, the contribution and the modifications, @Cologler! I'll try to bundle up a release of TinyDB next week 🙂

@msiemens

This comment has been minimized.

Copy link
Owner

msiemens commented Aug 20, 2018

I've now released v3.11 🙂 I've also changed the parameter name to storage_proxy_class for consistency

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
2 participants
You can’t perform that action at this time.