Functional test - Add datastore query tests

[dhermes]

This implements projection, offset and group_by on datastore.query.Query. I looked and didn't see any equivalent bugs from them.

As new Query features are outside the scope of functional / regression testing, I am going to peel them off into a separate PR and then rebase this one onto it after that is reviewed. However, please feel free to dig into this review.

Some other things to notice:

• The value of more_results from storage.Connection.run_query should not be thrown away. #280 was filed as a result of this.

• In gcloud-node, fetching on a query returns a new query with the cursor set. We set the cursor after a query is completed, but this is not start_cursor so a user is still required to create a new cursor via:

  results = query.fetch()
  new_query = query.with_cursor(query.cursor())

  Also note that in tests like test_limit_queries The value of more_results from storage.Connection.run_query should not be thrown away. #280 is most relevant and the gcloud-node implementation could also benefit from the more_results boolean returned from a connection.

• Raising a RuntimeError on cursor() access feels wrong. In reality, we'd like to access the start cursor and the end cursor and the one set on Query is one we next really want set, but instead returned as part of fetch results.

• Running these tests very often (i.e. 3 times a minute) resulted in very flaky results for the filter tests. I tried to address this by doing bulk adds / deletes in a transaction but it still was flaky. /cc @pcostell

• The gcloud-node implementation seems to allow Dataset.save to take a list of entities and save them in a transaction. This may be worth adding.

• We haven't implemented keys_only queries though I'm unsure if the Cloud Datastore API supports this.

• Getting just the dict from two Entitys for comparison should be easier. I suppose we could implement __eq__ here.

• The code in test_query_paginate_with_offset is much more convoluted than the gcloud-node equivalent. This is mostly because runQuery does a lot under the covers in node land.

• I am unclear why the group_by query only returns two entities. Maybe someone can fill me in?

[dhermes]

@silvolu Seems like a transient failure but seeing the same issue on #282

[dhermes]

@tseaver Rebased this PR on top of #282 to make the relationship more clear.

[pcostell]

Running these tests very often (i.e. 3 times a minute) resulted in very flaky results for the filter tests. I tried to address this by doing bulk adds / deletes in a transaction but it still was flaky. /cc @pcostell
The gcloud-node implementation seems to allow Dataset.save to take a list of entities and save them in a transaction. This may be worth adding.

The issue is that queries are eventually consistent. We've found that for testing queries it is easiest to just put all of the test data into a single entity group. Then you can run an ancestor query on them.

We haven't implemented keys_only queries though I'm unsure if the Cloud Datastore API supports this.

Datastore just exposes this as a projection of __key__. I think this is probably the better way to expose it as it will allow users to project both __key__ and other properties.

I am unclear why the group_by query only returns two entities. Maybe someone can fill me in?

GROUP BY is designed to only return one result per grouped value. So you'll get one result for alive = True and one for alive = False. Really, you should never be allowed to project more values than you've group_by'd (done effectively here as a SELECT *). This can be used to implement SELECT DISTINCT if your projection is equal to your group by. Then you'd expect to see two results, True and False. Over-projecting will in this case return a random (not really random, but based on sort order) entity in this list. We plan to support MIN/MAX in the backend so that you can say, for example,

SELECT alive, MAX(occurrences) FROM Character GROUP BY alive

This would let you see if a dead character or alive character appeared in more episodes. (Also, spoilers?!?!)

[pcostell]

The code in test_query_paginate_with_offset is much more convoluted than the gcloud-node equivalent. This is mostly because runQuery does a lot under the covers in node land.

I think actually it's better to let the user do more work here. I believe the normal use case for pagination is something like this:

1. User goes to website

2. We run:

   curr_cursor = request.get('next_page', None)
   query = makeQuery().limit(page_size)
   if curr_cursor
    query.start_cursor(curr_cursor)
   results = query.fetch()
   next_page_link = url + "&next_page=" + query.cursor()
   prev_page_link = url + "&prev_page=" + curr_cursor

3. Return list to user with prev and next page

In this case, there isn't much benefit to returning the next query to run to the user as they are going to need to reconstruct it themselves in a separate request.

[dhermes]

@pcostell I was referring to paging with offsets instead of cursors, which is obviously frowned upon.

As for the ancestor query, this doesn't seem to be done in the node tests. Are those tests flaky?

Also, what is the recommended way to implement ancestor queries. It seems we'll need to do this before using them.

[tseaver]

Query objects support filtering by ancestor, E.g., to clear items from a parent::

from gcloud.datastore.key import Key
from gcloud.datastore.query import Query
query = Query(datastore, 'Item')
parent = Key.from_path(['Parent', 'parent_name'])
for item in query.ancestor(parent.key()).fetch():
    item.delete()

[tseaver]

Travis failiure:

lint runtests: commands[1] | python run_pylint.py
************* Module regression.datastore
E:199,20: Unexpected keyword argument 'dataset' in constructor call (unexpected-keyword-arg)
E:263,22: Unexpected keyword argument 'dataset' in constructor call (unexpected-keyword-arg)
Pylint failed on test and demo code with status 2.

'regression/datastore.py' needs to sync w/ #276 (keys no longer take a
'dataset' argument in 'init', 'from_path', or 'from_protobuf').

[coveralls]

Coverage remained the same when pulling 15cd053 on dhermes:functional-test-part2 into 2da8654 on GoogleCloudPlatform:master.

[coveralls]

Coverage remained the same when pulling 15cd053 on dhermes:functional-test-part2 into 2da8654 on GoogleCloudPlatform:master.

[coveralls]

Coverage remained the same when pulling 621b0e9 on dhermes:functional-test-part2 into 2da8654 on GoogleCloudPlatform:master.

[dhermes]

@pcostell I reworked this code to use queries on data that had persisted for a long time instead of querying immediately after insert.

Running this twice in Travis, both filter queries (test_query_multiple_filters and test_query_simple_filter) failed:
https://travis-ci.org/dhermes/gcloud-python/builds/38797026#L290-L303
https://travis-ci.org/dhermes/gcloud-python/builds/38796835#L460-L473

Is there something I'm missing on the node side? They don't seem to have issues passing
https://travis-ci.org/GoogleCloudPlatform/gcloud-node/builds/38839607#L581-L582
https://travis-ci.org/GoogleCloudPlatform/gcloud-node/builds/38657797#L584-L585

It seems these empty queries (i.e. 0 != 4, for the expected number of filtered results) are due to a "tablet that isn't hot", but my understanding of HRD is very likely outdated.

Also, FWIW I reworked to make them all ancestor queries under Eddard and they gave the same transient / flaky failures (on my local machine).

[coveralls]

Coverage remained the same when pulling 972a796 on dhermes:functional-test-part2 into 09859db on GoogleCloudPlatform:master.

[pcostell]

Interesting. You're almost certainly guaranteed to hit eventual consistency problems. I'm not sure why the node ones don't fail, perhaps they've just gotten lucky so far.

If you're issuing ancestor queries, you are guaranteed to get strong consistency. So I think the flakiness might be coming from somewhere else.

When you say you are running locally, do you mean issuing commands from your local computer to a production Cloud Datastore dataset? Or are you also running locally using gcd? If you use gcd you can set the consistency using --consistency=1.0 so that all queries are strongly consistent. You could use this to determine if your queries are flaky due to eventual consistency or some other reason.

[dhermes]

Sorry, I meant running on my local machine (via tox -e regression with this PR) but targeting a live Cloud Datastore dataset.

I'll double-check on the flakiness with ancestor queries and commit it / point you at the commit if I can reproduce the flakiness.