-
Notifications
You must be signed in to change notification settings - Fork 1.5k
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
Updating datastore HTTP wrapper. #4388
Conversation
- Making the method arguments (both in position and name) match the arguments for `google.cloud.datastore_v1.gapic.datastore_client.DatastoreClient` - Passing positional as positional and keyword as keyword when using the low-level API client in `Client.get()` (was previously using all keyword arguments to `.lookup()`, which caused this issue) - Updating mock call assertions to match the change in calling behavior. Fixes googleapis#4387.
When I checked the
|
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.
Ugh, it feeds a little B&D / un-Pythonic to insist on "positional-only" args. Having the underlying bits change their argument names is actually a bit of a foul. I guess this is "not user-facing", so we can go ahead.
@tseaver I think it's "good form" to do. With the addition of keyword-only arguments in Python 3, I'm not so sure that's a common attitude. From my perspective, using positional as positional and keyword as keyword actually indicates that the knows that. Also, this isn't the first time we've been bitten by the auto-generated interface changing, which is why I advocate for "strict" adherence the offered interface. PS What's B&D? |
@dhermes note that I approved the PR: I'm just "arguing with the ref."
They are equally capable of breaking this usage (changing order of "positional" arguments, for instance, which would be covered by using only keyword args when calling them). B&D -> "bondage-and-discipline". |
- Making the method arguments (both in position and name) match the arguments for `google.cloud.datastore_v1.gapic.datastore_client.DatastoreClient` - Passing positional as positional and keyword as keyword when using the low-level API client in `Client.get()` (was previously using all keyword arguments to `.lookup()`, which caused this issue) - Updating mock call assertions to match the change in calling behavior. Fixes googleapis#4387.
- Making the method arguments (both in position and name) match the arguments for `google.cloud.datastore_v1.gapic.datastore_client.DatastoreClient` - Passing positional as positional and keyword as keyword when using the low-level API client in `Client.get()` (was previously using all keyword arguments to `.lookup()`, which caused this issue) - Updating mock call assertions to match the change in calling behavior. Fixes googleapis#4387.
google.cloud.datastore_v1.gapic.datastore_client.DatastoreClient
Client.get()
(was previously using all keyword arguments to.lookup()
, which caused this issue)Fixes #4387.