Query index in couchdb and support dict syntax: _id in database #369

tiangolo · 2018-04-05T12:02:16Z

Tick to sign-off your agreement to the Developer Certificate of Origin (DCO) 1.1
You have added tests for any code changes
You have updated the CHANGES.md
You have completed the PR template below:

What

I added 2 things, I needed both of them at the same time so I ended up doing a single PR instead of 2, sorry for that.

Move create_query_index from CloudantDatabase to CouchDatabase as the JSON indexes for Mango queries are now part of CouchDB.
Add a feature to allow checking if a document exists in a DB without having to resort to try-catch blocks, using standard dict syntax:

_id = 'julia30'
doc_exists = _id in my_database

if doc_exists:
    print('doc with _id julia30 exists in DB')

How

create_query_index: move the method from one class to the other. Move the tests from Cloudant specific to CouchDB.
Add a dict override for __contains__ that checks in the remote DB if the document exists.

Testing

Create a CouchDatabase object and call the create_query_index the same way you would for a Cloudant database.

Issues

I didn't create an issue before solving it.

emlaver

Moving the Mango index related methods to CouchDatabase requires our Travis CI to run against CouchDB 2.x. Required steps before further review of this PR:

Updating the Travis environment (.travis.yml) to use couchdb:2.1.1 or latest docker image
Fixing/updating test cases that fail/error against CouchDB 2.x.

Also, I've done a small test and it seems that moving the Mango index related methods to CouchDatabase doesn't break the API for existing users who call at these methods in CloudantDatabase. Please test and confirm.

emlaver · 2018-04-09T15:03:05Z

docs/getting_started.rst

+
+.. code-block:: python
+
+    doc_exists = 'julia30' in my_database


A test case is required for this change.

Done ✔️

tiangolo · 2018-04-11T14:36:46Z

Thanks for the review @emlaver

About setting CouchDB to the latest Docker image, I don't see any documentation on how to specify the version of CouchDB in Travis using simple, standard Travis with services: https://docs.travis-ci.com/user/database-setup/#CouchDB

I know it could be quite simpler to with Docker.

And I see there are branch names in this repo related to migrating the tests to Docker.

Do you need help with some of that? I have "some" experience with Docker, but I don't want to do stuff that doesn't align with the plans of the team.

For example, this project is configured to run tests for a full app in Docker. And run them on Travis (and GitLab CI, etc. It's quite simple to change the CI system once it's all in Docker).

emlaver · 2018-04-11T15:06:05Z

@tiangolo I'm currently working in a branch to use Docker to run our tests against the 2.1.1 version. I'll open a PR and our team with review when it's ready.

tiangolo · 2018-04-11T15:31:40Z

Excellent, thanks for reporting back @emlaver .

BTW, I'm actually running my local tests against a Docker (Docker Compose) CouchDB. Please let me know if there's anything I can help with that.

And let me know if there's anything still missing here (in this PR) that should be fixed before your PR is ready (I already added the missing tests).

emlaver · 2018-04-24T14:10:47Z

@tiangolo I've merged the CouchDB 2.1.1 testing PR. Please rebase your PR.

tiangolo · 2018-04-26T14:38:34Z

Thanks @emlaver , that's great! 🎉

Done, rebased (also fixed an issue I had there) ✔️

tiangolo · 2018-05-02T10:44:00Z

Hi @emlaver , do you have any news about this? 😁

emlaver · 2018-05-02T16:27:09Z

src/cloudant/database.py

+
+        :param str key: Document id used to check if it exists in the database.
+
+        :returns: A boolean, True if the document exists in the local or remote


This can be simplified to: True if ..., otherwise False.

emlaver · 2018-05-02T16:32:19Z

src/cloudant/database.py

+        Overrides dictionary __contains__ behavior to check if a document
+        by key exists in the current cached or remote database.
+
+        This allows using checks like


Replace with For example: to be consistent with other code blocks in comments.

smithsz

LGTM

ricellis · 2018-05-03T10:10:12Z

CHANGES.md

@@ -1,5 +1,7 @@
 # Unreleased

+- [NEW] Moved `create_query_index` to `CouchDatabase` as it is now implemented there.


Two things:
i) There are a bunch of other index/query related methods that moved as well.
ii) I know you are referring to new versions of CouchDB, but a newbie might not in which case "implemented there" is a bit confusing because it could mean CouchDatabase.

How about:

- [NEW] Moved `create_query_index` and other query related methods to `CouchDatabase` as the `_index`/`_find` API is available in CouchDB 2.x.

emlaver

Looks good, please squash your commits down to one commit. You can use your CHANGES entry and any additional info. as the commit message.

…t for 'key in db' [NEW] Moved `create_query_index` and other query related methods to `CouchDatabase` as the `_index`/`_find` API is available in CouchDB 2.x. [NEW] Added functionality to test if a key is in a database as in `key in db`, overriding dict `__contains__` and checking in the remote database.

tiangolo · 2018-05-06T10:30:04Z

Thanks @emlaver , done ✅

tiangolo · 2018-05-07T14:49:24Z

🎉 Thanks!

emlaver · 2018-05-07T15:09:01Z

@tiangolo Thank you for your work!

emlaver added this to New in python-cloudant Triage via automation Apr 9, 2018

ricellis moved this from New to Review in python-cloudant Triage Apr 10, 2018

emlaver suggested changes Apr 10, 2018

View reviewed changes

emlaver reviewed May 2, 2018

View reviewed changes

smithsz approved these changes May 3, 2018

View reviewed changes

ricellis suggested changes May 3, 2018

View reviewed changes

ricellis approved these changes May 3, 2018

View reviewed changes

emlaver approved these changes May 3, 2018

View reviewed changes

emlaver merged commit 9817315 into cloudant:master May 7, 2018

python-cloudant Triage automation moved this from Review to Done May 7, 2018

tiangolo deleted the query_index_in_couchdb branch May 7, 2018 14:49

ricellis added this to the 2.next milestone May 15, 2018

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Query index in couchdb and support dict syntax: _id in database #369

Query index in couchdb and support dict syntax: _id in database #369

tiangolo commented Apr 5, 2018

emlaver left a comment

emlaver Apr 9, 2018

tiangolo Apr 11, 2018

tiangolo commented Apr 11, 2018

emlaver commented Apr 11, 2018

tiangolo commented Apr 11, 2018

emlaver commented Apr 24, 2018

tiangolo commented Apr 26, 2018

tiangolo commented May 2, 2018

emlaver May 2, 2018

tiangolo May 3, 2018

emlaver May 2, 2018

tiangolo May 3, 2018

smithsz left a comment

ricellis May 3, 2018

tiangolo May 3, 2018

emlaver left a comment •

edited

tiangolo commented May 6, 2018

tiangolo commented May 7, 2018

emlaver commented May 7, 2018


		:param str key: Document id used to check if it exists in the database.

		:returns: A boolean, True if the document exists in the local or remote

		@@ -1,5 +1,7 @@
		# Unreleased

		- [NEW] Moved `create_query_index` to `CouchDatabase` as it is now implemented there.

Query index in couchdb and support dict syntax: _id in database #369

Query index in couchdb and support dict syntax: _id in database #369

Conversation

tiangolo commented Apr 5, 2018

What

How

Testing

Issues

emlaver left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

tiangolo commented Apr 11, 2018

emlaver commented Apr 11, 2018

tiangolo commented Apr 11, 2018

emlaver commented Apr 24, 2018

tiangolo commented Apr 26, 2018

tiangolo commented May 2, 2018

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

smithsz left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

emlaver left a comment • edited

Choose a reason for hiding this comment

tiangolo commented May 6, 2018

tiangolo commented May 7, 2018

emlaver commented May 7, 2018

emlaver left a comment •

edited