Introduce 'call_settings' parameter#70
Conversation
| invalid to have a key for a method that is not bundling-enabled. A | ||
| value of None indicates that the method in question should not | ||
| bundle. | ||
| client_config (dict or :class:`google.gax.CallSettings`): |
This comment was marked as spam.
This comment was marked as spam.
Sorry, something went wrong.
|
So I think my code had a confusion about the JSON-parsed config vs the return value of |
|
I still think that this means that to override any bundling/retry/page streaming behavior, even for a single method, you must provide an overly complex dictionary over every method in the API, and the keys of that dict must include non-configurable information like PageDescriptor. |
|
You're right about the bundling descriptors and page descriptors. I'm still suspicious about retrying_overrides and bundling_overrides though. With this client_config param, users can simply modify the input dict() (or JSON files) directly instead of passing override parameters, and I think we can omit them from the constructor parameters. |
|
I agree that we should not have retrying_overrides or bundling_overrides, and should provide a single override parameter instead. What I am unclear about is the structure of the dictionary being passed in. In my mind, it should
|
|
Hm, I see. So you are saying to allow call_settings specify a part of methods, only for customized ones. Uploaded a new one, which essentially invokes construct_settings() twice so that it can fall back to the default config. Or, probably fix construct_settings interface? Any ideas? |
|
Yes, I think this would require a change to I think there's also some more subtlety to the merging part of this setup that needs to be addressed in GAX. (For example, suppose the user defines a new |
Based on the discussion of googleapis#95 and googleapis/gapic-generator#70 -- this changes the interface of construct_settings a bit. The new interface does not have the override parameters for retrying and bundling anymore. Instead, it has a single config_override that has the same strucure as client_config has. API users can specify a copy of default config data with a few modifications, but if the user-supplied config does not specify some data, this will fall back to the default config, so users can safely supply a part of the client config, just for their edits.
|
Created a change on GAX to address the issue. PTAL. |
* Update construct_settings overrides Based on the discussion of #95 and googleapis/gapic-generator#70 -- this changes the interface of construct_settings a bit. The new interface does not have the override parameters for retrying and bundling anymore. Instead, it has a single config_override that has the same strucure as client_config has. API users can specify a copy of default config data with a few modifications, but if the user-supplied config does not specify some data, this will fall back to the default config, so users can safely supply a part of the client config, just for their edits. * Style fix. This needs to split override_config into a few functions due to code complexity warning by PEP8. * Update the overriding strategy. Based on the discussion, now overriding config can override the specified methods only, and does not merge the configurations between the default ones and the user-provided ones. * More merges of method configs. * Fixes
This removes retrying_overrides and bundling_overrides, because I think it's quite easy to modify the parameters before invoking the constructor. This fixes googleapis/gax-python#95 and googleapis/gax-ruby#5
|
When you have a chance, could you update this to reflect the changes merged into GAX? |
|
Updated for the new construct_settings behavior. PTAL |
| value of None indicates that the method in question should not | ||
| bundle. | ||
| client_config (dict): | ||
| A dictionary for call settings for each method. See |
This comment was marked as spam.
This comment was marked as spam.
Sorry, something went wrong.
This comment was marked as spam.
This comment was marked as spam.
Sorry, something went wrong.
This comment was marked as spam.
This comment was marked as spam.
Sorry, something went wrong.
|
Just an optional comment on updating a doc reference, but I don't want to block on this. LGTM |
* Introduce 'call_settings' parameter This removes retrying_overrides and bundling_overrides, because I think it's quite easy to modify the parameters before invoking the constructor. This fixes googleapis/gax-python#95 and googleapis/gax-ruby#5 * Fall back to the default config * Fix the new client_config param behavior * s/call settings/call options/
2 participants
This removes retrying_overrides and bundling_overrides, because
I think it's quite easy to modify the parameters before invoking
the constructor.
This fixes googleapis/gax-python#95 and googleapis/gax-ruby#5