-
Notifications
You must be signed in to change notification settings - Fork 65
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
Sort out Query options. #60
Conversation
Make use of the ``default_options`` argument to the ``Query`` constructor. Make all arguments to ``Query.fetch`` explicit, and perform the complicated dance of merging all the arguments and options to both the ``Query`` constructor and ``Query.fetch`` into a single set of options for passing to ``_datastore_query.fetch``.
|
||
class QueryOptions: | ||
__slots__ = ( | ||
"client", |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Not used in NDB.
|
||
class QueryOptions: | ||
__slots__ = ( | ||
"client", | ||
# Query options |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I've tried to organize these according to which ones are passed to the Query
constructor and which ones are passed to Query.fetch
. projection
is passed to both.
if config is not None and not isinstance(config, QueryOptions): | ||
raise TypeError("Config must be a QueryOptions instance.") | ||
|
||
for key in self.__slots__: |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Refactored to avoid AttributeError when accessing an attribute that wasn't explicitly passed in.
default = getattr(config, key, None) if config else None | ||
setattr(self, key, kwargs.get(key, default)) | ||
|
||
def __eq__(self, other): |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
To support unit tests.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looking good. Really a lot simpler than before.
Make use of the
default_options
argument to theQuery
constructor. Make all arguments to
Query.fetch
explicit, and performthe complicated dance of merging all the arguments and options to both
the
Query
constructor andQuery.fetch
into a single set ofoptions for passing to
_datastore_query.fetch
.