gax-python
gax-python copied to clipboard
googleapis
Rename and refactor CallSettings
Currently we have CallSettings and CallOptions, and it's hard to say their roles and the differences.
Actually CallSettings is the settings (or configurations) for an API call (or a method), such as the page descriptors or bundling descriptors, retry parameters.
On the other hand, CallOptions is the optional data to modify the behavior of individual invocation of the methods.
To me,
- CallSettings isn't a very clear name. 'Call-' prefix sounds like per-call (per-invocation) thing. Maybe,
MethodSettings? - the 'merge' method to create a new call settings doesn't look a good design after the redesign of create_api_call. I feel like CallSettings (or MethodSettings) should hold the default CallOptions instance, and CallOptions instance should have the
merge()method to create the actual options for the invocation.
Thoughts?
I think this would be a good task to starting up for @Landrito. This is for gax-python, but the same discussion must be applied to gax-ruby/nodejs as well (because we ported the python design to them).
In PHP, we just created CallSettings. I had this same thought about the confusion of CallOptions and CallSettings. In a case where you want both as classes, I might suggest DefaultCallSettings and CallSettings.
If you had MethodSettings + CallSettings, I don't think the relationship would be clear.
I'm afraid that DefaultCallSettings with CallSettings wouldn't suggest the right relationship between them. It sounds like DefaultCallSettings is a subclass of CallSettings -- but they are not.
My suggestion is to have MethodSettings (which holds the static properties of methods, such as page-streaming) with CallOptions (which holds updatable values), different prefixes and suffixes, and that would make things clearer.
I want to change the prefix because (to me) 'Call-' prefix seems to specify something for per-invocation. But the values for CallSettings are to determine the structure of the API call before the actual invocation happens (such as page-streaming or bundling or not).
Idea from @bjwatson: simply consolidate them into a single class, and expose the APIs (setter/getter) to operate internal data?
The smallest possible action we can take for this quarter is to prepend CallSettings with an underscore: CallSettings -> _CallSettings. That way no one will consider this to be a breakable surface.
This also affects Ruby.
Actually, re-open because we still may wish to evaluate a future renaming. But because it is no longer exposed to users, downgrade priority to P3.
