Skip to content

Conversation

@mdumandag
Copy link
Contributor

@mdumandag mdumandag commented Oct 16, 2020

This is the port of #212
to the 4.0 branch. However, there are some differences from that PR.

  • Documentation of the feature is a little bit different since the client
    side sorting is removed.
  • Tests are more-or-less ported directly with a few additions for
    input validations. However, the above PR was using six.assertCounEqual
    for assertions which checks equality but not order. For paging predicate
    tests, it makes more sense to check ordering.
  • Client side codecs and holder classes for PagingPredicates and AnchorDataLists
    are added.
  • Some public methods, like get_anchor, set_anchor, get_nearest_anchor_entry
    is removed. get_anchor is defined on the public API on the Java side but I think
    it does not provide any functionality to the user.

Also, some missing six.moves.range imports are added.

Protocol PR: hazelcast/hazelcast-client-protocol#355

This is the port of hazelcast#212
to the 4.0 branch. However, there are some differences from that PR.

- Documentation of the feature is a little bit different since the client
side sorting is removed.
- Tests are more-or-less ported directly with a few additions for
input validations. However, the above PR was using `six.assertCounEqual`
for assertions which checks equality but not order. For paging predicate
tests, it makes more sense to check ordering.
- Client side codecs and holder classes for PagingPredicates and AnchorDataLists
are added.
- Some public methods, like `get_anchor`, `set_anchor`, `get_nearest_anchor_entry`
is removed. `get_anchor` is defined on the public API on the Java side but I think
it does not provide any functionality to the user.
Copy link

@puzpuzpuz puzpuzpuz left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

@mdumandag
Copy link
Contributor Author

@puzpuzpuz , I agree with the comments you have made. I copy-pasted almost all of them from the Java reference manual, I will also send a pull request there.

@mdumandag
Copy link
Contributor Author

Thanks for the quick review

@mdumandag mdumandag merged commit 2292776 into hazelcast:master Nov 6, 2020
@mdumandag mdumandag deleted the paging-predicate branch November 6, 2020 13:22
mdumandag added a commit to mdumandag/hazelcast-reference-manual that referenced this pull request Nov 6, 2020
There were some problems in that chapter. Namely,

- Explanation of the code sample was not clear. For example, ``values``
should refer to the method call, but it was used in plural form, probably
refering the returned collection.
-  If the comparator is not set, keys or values of the collection should
be comparable, not the collection itself.
- The note about the transactional context is not related to the paragraphs
above. It makes sense to convert it into a note.

Credit goes to Andrey for spotting these problems on hazelcast/hazelcast-python-client#232
mdumandag added a commit to mdumandag/hazelcast-reference-manual that referenced this pull request Nov 6, 2020
There were some problems with that chapter. Namely,

- Explanation of the code sample was not clear. For example, ``values``
should refer to the method call, but it was used in plural form, probably
referring to the returned collection.
-  If the comparator is not set, keys or values of the collection should
be comparable, not the collection itself.
- The note about the transactional context is not related to the paragraphs
above. It makes sense to convert it into a note.

Credit goes to Andrey for spotting these problems on hazelcast/hazelcast-python-client#232
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants