Add offset alias property to be used in query_method. #76

Open
wants to merge 3 commits into
from

2 participants

@sstriker

Some clients require more traditional offset, limit functionality. For example an implementation of a dojo Store on top of an endpoints API (dojo-endpoints). As this store needs to stick to a set API, it can't use the more efficient nextPageToken approach.

This patch adds an offset to _EndpointsQueryInfo and uses it when calling fetch_page.

@sstriker sstriker Add offset alias property to be used in query_method.
* endpoints_proto_datastore/ndb/model.py
  (_EndpointsQueryInfo.__init__): Add _offset member.
  (_EndpointsQueryInfo._GetOffset, _SetOffset, offset): Add offset property.
  (EndpointsModel.OffsetSet, offset): Add offset property.
  (EndpointsModel.query_method.QueryFromRequestMethod): Add offset to
     query_options which is passed to fetch_page.
492a3b6
@dhermes dhermes and 1 other commented on an outdated diff Jul 22, 2013
endpoints_proto_datastore/ndb/model.py
+
+ Args:
+ value: A potential value for a offset.
+
+ Raises:
+ AttributeError: if query on the object is already final.
+ AttributeError: if the offset has already been set.
+ TypeError: if the value to be set is not a positive integer.
+ """
+ if self._query_final is not None:
+ raise AttributeError('Can\'t set offset. Query info is final.')
+
+ if self._offset is not None:
+ raise AttributeError('Offset can\'t be set twice.')
+ if not isinstance(value, (int, long)) or value < 0:
+ raise TypeError('Offset must be a positive integer.')
@dhermes
Google Cloud Platform member
dhermes added a line comment Jul 22, 2013

0 is acceptable given the if statement, but 0 is not a positive integer. Do you want positive or non-negative?

@sstriker
sstriker added a line comment Jul 22, 2013

non-negative. I'll fix the comment.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
@dhermes dhermes commented on the diff Jul 22, 2013
endpoints_proto_datastore/ndb/model.py
@@ -1494,7 +1546,10 @@ def QueryFromRequestMethod(service_instance, request):
raise endpoints.ForbiddenException(
QUERY_MAX_EXCEEDED_TEMPLATE % (request_limit, limit_max))
- query_options = {'start_cursor': query_info.cursor}
+ query_options = {
+ 'start_cursor': query_info.cursor,
@dhermes
Google Cloud Platform member
dhermes added a line comment Jul 22, 2013

Is there any sort of issue if cursor and offset are used simultaneously?

@sstriker
sstriker added a line comment Jul 22, 2013

Fair question. I'll add some protection so you can't set both.

@dhermes
Google Cloud Platform member
dhermes added a line comment Oct 2, 2013

@sstriker Did this ever get added?

@sstriker
sstriker added a line comment Oct 2, 2013

@dhermes Assuming this refers to the protection against using cursor and offset simultaneously, that should be in the updated pull request.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
sstriker added some commits Jul 23, 2013
@sstriker sstriker Fix comments regarding accepted values for offset. Protect against se…
…tting both cursor and offset.

* endpoints_proto_datastore/ndb/model.py
  (_EndpointsQueryInfo.__init__): Fix comment.
  (_EndpointsQueryInfo._SetCursor, _SetOffset): Prevent setting of both cursor
     and offset.  Fix comment.
c7b4e76
@sstriker sstriker Fix comments regarding raised exceptions in relation to accepted values
for offset and cursor.

* endpoints_proto_datastore/ndb/model.py
  (_EndpointsQueryInfo._SetCursor, _SetOffset): Add comment regarding
   AttributeError raised when both both cursor and offset are attempted
   to be set.
207816c
@sstriker

Updated pull request; I think this addresses all outstanding questions/comments.

@dhermes
Google Cloud Platform member

@sstriker I just realized this is orphaned.

Still worth doing? Been addressed by a different fix?

@sstriker
@dhermes
Google Cloud Platform member

Good to hear.

Can you rebase to HEAD in master and then I'll review and we can get it in?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment