Repository: googleapis/gax-python

Issue:
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?

Discussion:
codecov-io: ## [Current coverage](https://codecov.io/gh/googleapis/gax-python/pull/125?src=pr) is 97.19% (diff: 100%)
> Merging [#125](https://codecov.io/gh/googleapis/gax-python/pull/125?src=pr) into [master](https://codecov.io/gh/googleapis/gax-python/branch/master?src=pr) will not change coverage

```diff
@@             master       #125   diff @@
==========================================
  Files             8          8          
  Lines           607        607          
  Methods           0          0          
  Messages          0          0          
  Branches          0          0          
==========================================
  Hits            590        590          
  Misses           17         17          
  Partials          0          0          
```

> Powered by [Codecov](https://codecov.io?src=pr). Last update [e8f6daa...009253d](https://codecov.io/gh/googleapis/gax-python/compare/e8f6daaecfcbbf01fc78a54542108295aaf7f356...009253dc3736831b9b7dd2f3cf55f72dbbae9d14?src=pr)
jmuk: LGTM

Patch:
diff --git a/google/gax/__init__.py b/google/gax/__init__.py
index 64eea72..09c38a5 100644
--- a/google/gax/__init__.py
+++ b/google/gax/__init__.py
@@ -48,7 +48,7 @@ CallOptions belongs will attempt to inherit that field from its default
 settings."""
 
 
-class CallSettings(object):
+class _CallSettings(object):
     """Encapsulates the call settings for an API call."""
     # pylint: disable=too-few-public-methods
     def __init__(self, timeout=30, retry=None, page_descriptor=None,
@@ -94,10 +94,10 @@ class CallSettings(object):
         return self.page_token is None
 
     def merge(self, options):
-        """Returns a new CallSettings merged from this and a CallOptions object.
+        """Returns new _CallSettings merged from this and a CallOptions object.
 
         Note that passing if the CallOptions instance specifies a page_token,
-        the merged CallSettings will have ``flatten_pages`` disabled. This
+        the merged _CallSettings will have ``flatten_pages`` disabled. This
         permits toggling per-resource/per-page page streaming.
 
         Args:
@@ -106,10 +106,10 @@ class CallSettings(object):
               object
 
         Returns:
-            A :class:`CallSettings` object.
+            A :class:`_CallSettings` object.
         """
         if not options:
-            return CallSettings(
+            return _CallSettings(
                 timeout=self.timeout, retry=self.retry,
                 page_descriptor=self.page_descriptor,
                 page_token=self.page_token,
@@ -142,7 +142,7 @@ class CallSettings(object):
                 kwargs = self.kwargs.copy()
                 kwargs.update(options.kwargs)
 
-            return CallSettings(
+            return _CallSettings(
                 timeout=timeout, retry=retry,
                 page_descriptor=self.page_descriptor, page_token=page_token,
                 bundler=bundler, bundle_descriptor=self.bundle_descriptor,
diff --git a/google/gax/api_callable.py b/google/gax/api_callable.py
index 955882e..622c08e 100644
--- a/google/gax/api_callable.py
+++ b/google/gax/api_callable.py
@@ -35,7 +35,7 @@ import time
 
 from future import utils
 
-from . import (BackoffSettings, BundleOptions, bundling, CallSettings, config,
+from . import (BackoffSettings, BundleOptions, bundling, _CallSettings, config,
                PageIterator, ResourceIterator, RetryOptions)
 from .errors import GaxError, RetryError
 
@@ -301,7 +301,7 @@ def construct_settings(
         service_name, client_config, config_override,
         retry_names, bundle_descriptors=None, page_descriptors=None,
         kwargs=None):
-    """Constructs a dictionary mapping method names to CallSettings.
+    """Constructs a dictionary mapping method names to _CallSettings.
 
     The ``client_config`` parameter is parsed from a client configuration JSON
     file of the form:
@@ -406,7 +406,7 @@ def construct_settings(
             _construct_retry(overriding_method, overrides.get('retry_codes'),
                              overrides.get('retry_params'), retry_names))
 
-        defaults[snake_name] = CallSettings(
+        defaults[snake_name] = _CallSettings(
             timeout=timeout, retry=retry,
             page_descriptor=page_descriptors.get(snake_name),
             bundler=bundler, bundle_descriptor=bundle_descriptor,
@@ -452,7 +452,7 @@ def create_api_call(func, settings):
 
     Args:
       func (callable[[object], object]): is used to make a bare rpc call
-      settings (:class:`CallSettings`): provides the settings for this call
+      settings (:class:`_CallSettings`): provides the settings for this call
 
     Returns:
       func (callable[[object], object]): a bound method on a request stub used


Test patch:
diff --git a/test/test_api_callable.py b/test/test_api_callable.py
index 5621619..f45c5f4 100644
--- a/test/test_api_callable.py
+++ b/test/test_api_callable.py
@@ -36,7 +36,7 @@ import unittest2
 
 from google.gax import (
     api_callable, bundling, BackoffSettings, BundleDescriptor, BundleOptions,
-    CallSettings, CallOptions, INITIAL_PAGE, PageDescriptor, RetryOptions)
+    _CallSettings, CallOptions, INITIAL_PAGE, PageDescriptor, RetryOptions)
 from google.gax.errors import GaxError, RetryError
 
 
@@ -116,19 +116,19 @@ class AnotherException(Exception):
 class TestCreateApiCallable(unittest2.TestCase):
 
     def test_call_api_call(self):
-        settings = CallSettings()
+        settings = _CallSettings()
         my_callable = api_callable.create_api_call(
             lambda _req, _timeout: 42, settings)
         self.assertEqual(my_callable(None), 42)
 
     def test_call_override(self):
-        settings = CallSettings(timeout=10)
+        settings = _CallSettings(timeout=10)
         my_callable = api_callable.create_api_call(
             lambda _req, timeout: timeout, settings)
         self.assertEqual(my_callable(None, CallOptions(timeout=20)), 20)
 
     def test_call_kwargs(self):
-        settings = CallSettings(kwargs={'key': 'value'})
+        settings = _CallSettings(kwargs={'key': 'value'})
         my_callable = api_callable.create_api_call(
             lambda _req, _timeout, **kwargs: kwargs['key'], settings)
         self.assertEqual(my_callable(None), 'value')
@@ -150,7 +150,7 @@ class TestCreateApiCallable(unittest2.TestCase):
                                  (to_attempt - 1) + [mock.DEFAULT])
         mock_call.return_value = 1729
         mock_time.return_value = 0
-        settings = CallSettings(timeout=0, retry=retry)
+        settings = _CallSettings(timeout=0, retry=retry)
         my_callable = api_callable.create_api_call(mock_call, settings)
         self.assertEqual(my_callable(None), 1729)
         self.assertEqual(mock_call.call_count, to_attempt)
@@ -163,7 +163,7 @@ class TestCreateApiCallable(unittest2.TestCase):
         mock_call.side_effect = CustomException('', _FAKE_STATUS_CODE_1)
         mock_time.return_value = 0
 
-        settings = CallSettings(timeout=0, retry=retry)
+        settings = _CallSettings(timeout=0, retry=retry)
         my_callable = api_callable.create_api_call(mock_call, settings)
         self.assertRaises(CustomException, my_callable, None)
         self.assertEqual(mock_call.call_count, 1)
@@ -179,7 +179,7 @@ class TestCreateApiCallable(unittest2.TestCase):
             BackoffSettings(0, 0, 0, 0, 0, 0, 1))
         mock_time.side_effect = [0, 2]
         mock_exc_to_code.side_effect = lambda e: e.code
-        settings = CallSettings(timeout=0, retry=retry)
+        settings = _CallSettings(timeout=0, retry=retry)
         my_callable = api_callable.create_api_call(fake_call, settings)
 
         try:
@@ -198,7 +198,7 @@ class TestCreateApiCallable(unittest2.TestCase):
         mock_call = mock.Mock()
         mock_call.side_effect = CustomException('', _FAKE_STATUS_CODE_1)
         mock_time.side_effect = ([0] * to_attempt + [2])
-        settings = CallSettings(timeout=0, retry=retry)
+        settings = _CallSettings(timeout=0, retry=retry)
         my_callable = api_callable.create_api_call(mock_call, settings)
 
         try:
@@ -219,7 +219,7 @@ class TestCreateApiCallable(unittest2.TestCase):
         mock_call = mock.Mock()
         mock_call.side_effect = CustomException('', _FAKE_STATUS_CODE_2)
         mock_time.return_value = 0
-        settings = CallSettings(timeout=0, retry=retry)
+        settings = _CallSettings(timeout=0, retry=retry)
         my_callable = api_callable.create_api_call(mock_call, settings)
         self.assertRaises(Exception, my_callable, None)
         self.assertEqual(mock_call.call_count, 1)
@@ -230,7 +230,7 @@ class TestCreateApiCallable(unittest2.TestCase):
         retry = RetryOptions(
             [_FAKE_STATUS_CODE_1],
             BackoffSettings(0, 0, 0, 0, 0, 0, 0))
-        settings = CallSettings(timeout=0, retry=retry)
+        settings = _CallSettings(timeout=0, retry=retry)
         my_callable = api_callable.create_api_call(lambda: None, settings)
 
         self.assertRaises(RetryError, my_callable, None)
@@ -258,7 +258,7 @@ class TestCreateApiCallable(unittest2.TestCase):
 
         params = BackoffSettings(3, 2, 24, 5, 2, 80, 2500)
         retry = RetryOptions([_FAKE_STATUS_CODE_1], params)
-        settings = CallSettings(timeout=0, retry=retry)
+        settings = _CallSettings(timeout=0, retry=retry)
         my_callable = api_callable.create_api_call(mock_call, settings)
 
         try:
@@ -315,7 +315,7 @@ class TestCreateApiCallable(unittest2.TestCase):
 
         with mock.patch('grpc.UnaryUnaryMultiCallable') as mock_grpc:
             mock_grpc.side_effect = grpc_return_value
-            settings = CallSettings(
+            settings = _CallSettings(
                 page_descriptor=fake_grpc_func_descriptor, timeout=0)
             my_callable = api_callable.create_api_call(
                 mock_grpc, settings=settings)
@@ -344,7 +344,7 @@ class TestCreateApiCallable(unittest2.TestCase):
                              expected)
 
     def test_bundling_page_streaming_error(self):
-        settings = CallSettings(
+        settings = _CallSettings(
             page_descriptor=object(), bundle_descriptor=object(),
             bundler=object())
         with self.assertRaises(ValueError):
@@ -362,7 +362,7 @@ class TestCreateApiCallable(unittest2.TestCase):
         def my_func(request, dummy_timeout):
             return len(request.elements)
 
-        settings = CallSettings(
+        settings = _CallSettings(
             bundler=bundler, bundle_descriptor=fake_grpc_func_descriptor,
             timeout=0)
         my_callable = api_callable.create_api_call(my_func, settings)
@@ -478,9 +478,9 @@ class TestCreateApiCallable(unittest2.TestCase):
             raise AnotherException
 
         gax_error_callable = api_callable.create_api_call(
-            abortion_error_func, CallSettings())
+            abortion_error_func, _CallSettings())
         self.assertRaises(GaxError, gax_error_callable, None)
 
         other_error_callable = api_callable.create_api_call(
-            other_error_func, CallSettings())
+            other_error_func, _CallSettings())
         self.assertRaises(AnotherException, other_error_callable, None)
diff --git a/test/test_gax.py b/test/test_gax.py
index 77220f8..74ea365 100644
--- a/test/test_gax.py
+++ b/test/test_gax.py
@@ -35,7 +35,7 @@ from __future__ import absolute_import
 import unittest2
 
 from google.gax import (
-    BundleOptions, CallOptions, CallSettings, INITIAL_PAGE, OPTION_INHERIT,
+    BundleOptions, CallOptions, _CallSettings, INITIAL_PAGE, OPTION_INHERIT,
     RetryOptions)
 
 
@@ -72,7 +72,7 @@ class TestCallSettings(unittest2.TestCase):
 
     def test_settings_merge_options1(self):
         options = CallOptions(timeout=46)
-        settings = CallSettings(timeout=9, page_descriptor=None, retry=None)
+        settings = _CallSettings(timeout=9, page_descriptor=None, retry=None)
         final = settings.merge(options)
         self.assertEqual(final.timeout, 46)
         self.assertIsNone(final.retry)
@@ -81,7 +81,7 @@ class TestCallSettings(unittest2.TestCase):
     def test_settings_merge_options2(self):
         retry = RetryOptions(None, None)
         options = CallOptions(retry=retry)
-        settings = CallSettings(
+        settings = _CallSettings(
             timeout=9, page_descriptor=None, retry=RetryOptions(None, None))
         final = settings.merge(options)
         self.assertEqual(final.timeout, 9)
@@ -92,8 +92,8 @@ class TestCallSettings(unittest2.TestCase):
         retry = RetryOptions(None, None)
         page_descriptor = object()
         options = CallOptions(timeout=46, page_token=INITIAL_PAGE)
-        settings = CallSettings(timeout=9, retry=retry,
-                                page_descriptor=page_descriptor)
+        settings = _CallSettings(timeout=9, retry=retry,
+                                 page_descriptor=page_descriptor)
         final = settings.merge(options)
         self.assertEqual(final.timeout, 46)
         self.assertEqual(final.page_descriptor, page_descriptor)
@@ -102,7 +102,7 @@ class TestCallSettings(unittest2.TestCase):
         self.assertEqual(final.retry, retry)
 
     def test_settings_merge_none(self):
-        settings = CallSettings(
+        settings = _CallSettings(
             timeout=23, page_descriptor=object(), bundler=object(),
             retry=object())
         final = settings.merge(None)