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

fix: Address queries not fully satisfying requested offset #18

Merged
merged 8 commits into from Apr 7, 2020

Conversation

crwilcox
Copy link
Contributor

@crwilcox crwilcox commented Mar 31, 2020

WIP, submitting tests to run on CI

Currently Failing Test: https://github.com/googleapis/python-datastore/pull/18/files#diff-8a7c6f378fc6b49d53199fcf42bcfe0fR518

Related to #20

@crwilcox crwilcox added the do not merge Indicates a pull request not ready for merge, due to either quality or timing. label Mar 31, 2020
@googlebot googlebot added the cla: yes This human has signed the Contributor License Agreement. label Mar 31, 2020
@crwilcox
Copy link
Contributor Author

crwilcox commented Mar 31, 2020

Test Added. Not really confirming the case. Will need to further flesh out.

def test_query_paginate_with_large_offset(self):
        page_query = self._base_query()
        page_query.order = "appearances"
        offset = 1000 # This is greater than the number of entries.
        limit = 3

        # Offset beyond items. Verify no items found
        iterator = page_query.fetch(limit=limit, offset=offset)
        page = six.next(iterator.pages)
        entities = list(page)
        self.assertEqual(len(entities), 0)

        # Offset beyond items, explicit start_cursor
        new_iterator = page_query.fetch(limit=limit, offset=offset, start_cursor=None)
        entities = list(new_iterator)
        self.assertEqual(len(entities), 0)

        # Offset beyond items, use cursor one item in.
        new_iterator = page_query.fetch(limit=1, offset=0)
        cursor = new_iterator.next_page_token
        new_iterator = page_query.fetch(limit=limit, offset=offset, start_cursor=cursor)
        entities = list(new_iterator)
        self.assertEqual(len(entities), 0)

@crwilcox crwilcox changed the title fix: datastore client not properly respecting limits and offsets. WIP: fix: datastore client not properly respecting limits and offsets. Apr 2, 2020
@crwilcox crwilcox force-pushed the large_offsets branch 2 times, most recently from 76817ee to 992d8ad Compare April 6, 2020 21:31
@crwilcox crwilcox changed the title WIP: fix: datastore client not properly respecting limits and offsets. fix: datastore client not properly respecting limits and offsets. Apr 6, 2020
@crwilcox crwilcox removed the do not merge Indicates a pull request not ready for merge, due to either quality or timing. label Apr 6, 2020
@crwilcox crwilcox changed the title fix: datastore client not properly respecting limits and offsets. fix: datastore client not properly respecting limits and offsets Apr 6, 2020
@crwilcox crwilcox changed the title fix: datastore client not properly respecting limits and offsets fix: Address queries not fully satisfying requested offset Apr 6, 2020
@crwilcox
Copy link
Contributor Author

crwilcox commented Apr 6, 2020

10 tests fail both with and without my changes

FAILED tests/system/test_system.py::TestDatastoreQuery::test_ancestor_query - AssertionError: 4 != 8
FAILED tests/system/test_system.py::TestDatastoreQuery::test_limit_queries - AssertionError: 4 != 5
FAILED tests/system/test_system.py::TestDatastoreQuery::test_ordered_query - AssertionError: 4 != 8
FAILED tests/system/test_system.py::TestDatastoreQuery::test_projection_query - AssertionError: 4 != 9
FAILED tests/system/test_system.py::TestDatastoreQuery::test_query_distinct_on - AssertionError: 1 != 2
FAILED tests/system/test_system.py::TestDatastoreQuery::test_query_key_filter - AssertionError: 0 != 1
FAILED tests/system/test_system.py::TestDatastoreQuery::test_query_multiple_filters - AssertionError: 3 != 4
FAILED tests/system/test_system.py::TestDatastoreQuery::test_query_paginate_with_offset - AssertionError: 2 != 3
FAILED tests/system/test_system.py::TestDatastoreQuery::test_query_paginate_with_start_cursor - AssertionError: 2 != 3
FAILED tests/system/test_system.py::TestDatastoreQuery::test_query_simple_filter - AssertionError: 4 != 6

@crwilcox crwilcox added the kokoro:force-run Add this label to force Kokoro to re-run the tests. label Apr 7, 2020
@crwilcox
Copy link
Contributor Author

crwilcox commented Apr 7, 2020

Update: after refreshing test dataset, all tests are passing.

@yoshi-kokoro yoshi-kokoro removed the kokoro:force-run Add this label to force Kokoro to re-run the tests. label Apr 7, 2020
@crwilcox crwilcox merged commit e7b5fc9 into master Apr 7, 2020
@crwilcox
Copy link
Contributor Author

crwilcox commented Apr 7, 2020

Bypassing kokoro. Only failures are lint and docs. On areas not touched by this change.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
cla: yes This human has signed the Contributor License Agreement.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants