-
Notifications
You must be signed in to change notification settings - Fork 1.6k
Functional test - Add datastore query tests #281
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
Conversation
fb70c26 to
99f00f3
Compare
99f00f3 to
1d1b147
Compare
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.
Datastore just exposes this as a projection of
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 This would let you see if a dead character or alive character appeared in more episodes. (Also, spoilers?!?!) |
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:
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. |
ccec7be to
f26dfc5
Compare
|
@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. |
|
Query objects support filtering by ancestor, E.g., to clear items from a parent:: |
|
Travis failiure: 'regression/datastore.py' needs to sync w/ #276 (keys no longer take a |
6bdc25d to
15cd053
Compare
|
@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 ( Is there something I'm missing on the It seems these empty queries (i.e. Also, FWIW I reworked to make them all ancestor queries under |
621b0e9 to
972a796
Compare
|
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. |
|
Sorry, I meant running on my local machine (via I'll double-check on the flakiness with ancestor queries and commit it / point you at the commit if I can reproduce the flakiness. |
* chore(python): add missing import for prerelease testing Source-Link: googleapis/synthtool@d2871d9 Post-Processor: gcr.io/cloud-devrel-public-resources/owlbot-python:latest@sha256:b2dc5f80edcf5d4486c39068c9fa11f7f851d9568eea4dcba130f994ea9b5e97 * check samples failure * 🦉 Updates from OwlBot post-processor See https://github.com/googleapis/repo-automation-bots/blob/main/packages/owl-bot/README.md * lint * check samples failure * add 30 seconds to request timestamp Co-authored-by: Owl Bot <gcf-owl-bot[bot]@users.noreply.github.com> Co-authored-by: Anthonios Partheniou <[email protected]>
) Co-authored-by: Anthonios Partheniou <[email protected]>
Co-authored-by: release-please[bot] <55107282+release-please[bot]@users.noreply.github.com>
Source-Link: googleapis/synthtool@d2871d9 Post-Processor: gcr.io/cloud-devrel-public-resources/owlbot-python:latest@sha256:b2dc5f80edcf5d4486c39068c9fa11f7f851d9568eea4dcba130f994ea9b5e97 Co-authored-by: Owl Bot <gcf-owl-bot[bot]@users.noreply.github.com>
…[autoapprove] (#281) Source-Link: googleapis/synthtool@1f37ce7 Post-Processor: gcr.io/cloud-devrel-public-resources/owlbot-python:latest@sha256:8e84e0e0d71a0d681668461bba02c9e1394c785f31a10ae3470660235b673086
* chore: add default_version and codeowner_team to .repo-metadata.json * update default_version
- [ ] Regenerate this pull request now. docs: list oneofs in docstring fix(deps): require google-api-core >= 1.28.0 fix(deps): drop packaging dependency fix: fix extras_require typo in setup.py committer: busunkim96@ PiperOrigin-RevId: 406468269 Source-Link: googleapis/googleapis@83d81b0 Source-Link: googleapis/googleapis-gen@2ff001f Copy-Tag: eyJwIjoiLmdpdGh1Yi8uT3dsQm90LnlhbWwiLCJoIjoiMmZmMDAxZmJhY2I5ZTc3ZTcxZDczNGRlNWY5NTVjMDVmZGFlODUyNiJ9
Co-authored-by: release-please[bot] <55107282+release-please[bot]@users.noreply.github.com>
* chore(deps): update all dependencies * 🦉 Updates from OwlBot post-processor See https://github.com/googleapis/repo-automation-bots/blob/main/packages/owl-bot/README.md * revert * revert Co-authored-by: Owl Bot <gcf-owl-bot[bot]@users.noreply.github.com> Co-authored-by: Anthonios Partheniou <[email protected]>
- [ ] Regenerate this pull request now. PiperOrigin-RevId: 394579113 Committer yuwangyw@ Source-Link: googleapis/googleapis@9c7eb1f Source-Link: googleapis/googleapis-gen@5934384 feat(v1): Add content type Relationship to support relationship search
…281) Source-Link: googleapis/synthtool@6b4d5a6 Post-Processor: gcr.io/cloud-devrel-public-resources/owlbot-python:latest@sha256:f792ee1320e03eda2d13a5281a2989f7ed8a9e50b73ef6da97fac7e1e850b149 Co-authored-by: Owl Bot <gcf-owl-bot[bot]@users.noreply.github.com>
Source-Link: googleapis/synthtool@f15cc72 Post-Processor: gcr.io/cloud-devrel-public-resources/owlbot-python:latest@sha256:bc5eed3804aec2f05fad42aacf973821d9500c174015341f721a984a0825b6fd
Source-Link: googleapis/synthtool@571ee2c Post-Processor: gcr.io/cloud-devrel-public-resources/owlbot-python:latest@sha256:660abdf857d3ab9aabcd967c163c70e657fcc5653595c709263af5f3fa23ef67
Co-authored-by: release-please[bot] <55107282+release-please[bot]@users.noreply.github.com>
Source-Link: googleapis/synthtool@56da63e Post-Processor: gcr.io/cloud-devrel-public-resources/owlbot-python:latest@sha256:993a058718e84a82fda04c3177e58f0a43281a996c7c395e0a56ccc4d6d210d7
…ve] (#281) Source-Link: https://togithub.com/googleapis/synthtool/commit/6ed3a831cb9ff69ef8a504c353e098ec0192ad93 Post-Processor: gcr.io/cloud-devrel-public-resources/owlbot-python:latest@sha256:3abfa0f1886adaf0b83f07cb117b24a639ea1cb9cffe56d43280b977033563eb
* chore(deps): Remove try/except for mock import * 🦉 Updates from OwlBot post-processor See https://github.com/googleapis/repo-automation-bots/blob/main/packages/owl-bot/README.md --------- Co-authored-by: Owl Bot <gcf-owl-bot[bot]@users.noreply.github.com>
This implements
projection,offsetandgroup_byondatastore.query.Query. I looked and didn't see any equivalent bugs from them.As new
Queryfeatures 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_resultsfrom 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 notstart_cursorso a user is still required to create a new cursor via:Also note that in tests like
test_limit_queriesThe value ofmore_resultsfrom storage.Connection.run_query should not be thrown away. #280 is most relevant and thegcloud-nodeimplementation could also benefit from themore_resultsboolean returned from a connection.Raising a
RuntimeErroroncursor()access feels wrong. In reality, we'd like to access the start cursor and the end cursor and the one set onQueryis one we next really want set, but instead returned as part offetchresults.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
transactionbut it still was flaky. /cc @pcostellThe
gcloud-nodeimplementation seems to allowDataset.saveto take a list of entities and save them in a transaction. This may be worth adding.We haven't implemented
keys_onlyqueries though I'm unsure if the Cloud Datastore API supports this.Getting just the
dictfrom twoEntitys for comparison should be easier. I suppose we could implement__eq__here.The code in
test_query_paginate_with_offsetis much more convoluted than thegcloud-nodeequivalent. This is mostly becauserunQuerydoes a lot under the covers in node land.I am unclear why the
group_byquery only returns two entities. Maybe someone can fill me in?