Repository navigation
Update the structure of api_callable. - #107
Conversation
Now this allows getting call options as an optional parameter and changes its behavior. In a part of googleapis#104 effort.
| @property | ||
| def flatten_pages(self): | ||
| """ | ||
| A boolean property whether a page streamed response should make |
There was a problem hiding this comment.
"property whether" --> "property indicating whether"
Current coverage is 97.16%@@ master #107 diff @@
==========================================
Files 8 8
Lines 581 600 +19
Methods 0 0
Messages 0 0
Branches 0 0
==========================================
+ Hits 565 583 +18
- Misses 16 17 +1
Partials 0 0
|
|
Note: googleapis/gapic-generator#214 also changes the handling of |
|
I reviewed #104 discussion but unclear why metadata goes into construct_settings. It's actually not discussed well. But I don't think that's the key part of the redesign, and therefore I think that's no problem to be in a part of method invocation, i.e. And this structure would work well with this PR. |
|
regarding metadata -- this was in Garrett's original mockup on the pubsub codegen, and is implicitly represented in #104 by the simplified method invocation in the second code block. If you feel that this should not be addressed in this PR, I can do it in a follow-up. |
| def next(self): | ||
| """Retrieves the next resource.""" | ||
| # pylint: disable=next-method-called | ||
| while not self._current: |
There was a problem hiding this comment.
Why the while loop? That is, when will a PageIterator contain a falsy value?
There was a problem hiding this comment.
PageIterator could return an empty array but have a next page token, in that case this should skip the empty array and fetch the next page.
|
Regarding metadata -- rethinking about that, I'm okay with merging into construct_settings. |
|
SGTM regarding |
|
Updated about the metadata -- not renamed the class yet. I think it can be done in another PR. Also, I think GAX should not be strongly tied to gRPC itself -- it should be encapsulated into grpc.py file only as far as possible, and therefore, I've added 'kwargs' argument instead. |
| page_descriptor=self.page_descriptor, page_token=page_token, | ||
| bundler=bundler, bundle_descriptor=self.bundle_descriptor) | ||
| bundler=bundler, bundle_descriptor=self.bundle_descriptor, | ||
| kwargs=self.kwargs) |
There was a problem hiding this comment.
Let's a actually do
if options.kwargs == OPTION_INHERIT:
kwargs = self.kwargs
else:
kwargs = self.kwargs.copy()
kwargs.update(options.kwargs)
I can't tell where options.kwargs is currently being used -- maybe we are just ignoring it at the moment?
There was a problem hiding this comment.
added -- no such options.kwargs exist currently, so I've added it.
|
Just one concern about the CallSettings |
|
LGTM. I will update googleapis/gapic-generator#214 to put metadata into kwargs. |
4 participants
Now this allows getting call options as an optional parameter
and changes its behavior.
In a part of #104 effort.