Repository: googleapis/python-api-core (python)

Issue:
Modified reserved word field names included in field masks.
I'm not sure whether this is an issue with this library, the [`generator`](https://github.com/googleapis/gapic-generator-python) or the [`field mask helper`](https://github.com/googleapis/python-api-core/blob/master/google/api_core/protobuf_helpers.py#L290), but it seems to be incorrect behavior that's blocking for Ads users, unless there's a workaround or other solution I'm not aware of:

#### Environment details

  - Programming language: `Python`
  - OS: `gLinux`
  - Language runtime version: `3.7.0`
  - Package version: `1.18.1`

#### Steps to reproduce

Assuming we have a protobuf definition like the following:

```
message HasType {
  string type = 1;
}
```

We'll have a generated Python class similar to:

```py
class HasType(proto.Message):
    type_ = proto.Field(proto.STRING, number=1,)
```

Where `type` has been modified to `type_`. 

Normally setting `type_` isn't a problem, the issue is that this modified field name gets leaked into field masks, which are revoked by the API because it doesn't match the protobuf definition. This prevents users from executing update requests where the `type` field on any object has been modified. 

Here's a short script to reproduce:

```py
from google.api_core import protobuf_helpers
import proto

class HasType(proto.Message):
    type_ = proto.Field(proto.STRING, number=1,)

ht = HasType()
ht.type_ = "has type"

fm = protobuf_helpers.field_mask(None, ht._pb)

print(fm.paths[0])
# >>> "type_"
```


Pull request description:
fix: strip trailing `_` from field mask paths
Thank you for opening a Pull Request! Before submitting your PR, there are a few things you can do to make sure it goes smoothly:
- [x] Make sure to open an issue as a [bug/issue](https://github.com/googleapis/python-api-core/issues/new/choose) before writing your code!  That way we can discuss the change, evaluate designs, and agree on the general idea
- [x] Ensure the tests and linter pass
- [x] Code coverage does not decrease (if any source code was changed)
- [x] Appropriate docs were updated (if necessary)

CC @benrkarl

Fixes #227🦕

Patch:
diff --git a/.github/.OwlBot.lock.yaml b/.github/.OwlBot.lock.yaml
index cb06536..9ee60f7 100644
--- a/.github/.OwlBot.lock.yaml
+++ b/.github/.OwlBot.lock.yaml
@@ -1,3 +1,3 @@
 docker:
   image: gcr.io/repo-automation-bots/owlbot-python:latest
-  digest: sha256:5ff7446edeaede81c3ed58b23a4e76a5403fba1350ce28478045657303b6479d
+  digest: sha256:aea14a583128771ae8aefa364e1652f3c56070168ef31beb203534222d842b8b
diff --git a/.kokoro/samples/python3.6/periodic-head.cfg b/.kokoro/samples/python3.6/periodic-head.cfg
index f9cfcd3..a18c0cf 100644
--- a/.kokoro/samples/python3.6/periodic-head.cfg
+++ b/.kokoro/samples/python3.6/periodic-head.cfg
@@ -7,5 +7,5 @@ env_vars: {
 
 env_vars: {
     key: "TRAMPOLINE_BUILD_FILE"
-    value: "github/python-pubsub/.kokoro/test-samples-against-head.sh"
+    value: "github/python-api-core/.kokoro/test-samples-against-head.sh"
 }
diff --git a/.kokoro/samples/python3.7/periodic-head.cfg b/.kokoro/samples/python3.7/periodic-head.cfg
index f9cfcd3..a18c0cf 100644
--- a/.kokoro/samples/python3.7/periodic-head.cfg
+++ b/.kokoro/samples/python3.7/periodic-head.cfg
@@ -7,5 +7,5 @@ env_vars: {
 
 env_vars: {
     key: "TRAMPOLINE_BUILD_FILE"
-    value: "github/python-pubsub/.kokoro/test-samples-against-head.sh"
+    value: "github/python-api-core/.kokoro/test-samples-against-head.sh"
 }
diff --git a/.kokoro/samples/python3.8/periodic-head.cfg b/.kokoro/samples/python3.8/periodic-head.cfg
index f9cfcd3..a18c0cf 100644
--- a/.kokoro/samples/python3.8/periodic-head.cfg
+++ b/.kokoro/samples/python3.8/periodic-head.cfg
@@ -7,5 +7,5 @@ env_vars: {
 
 env_vars: {
     key: "TRAMPOLINE_BUILD_FILE"
-    value: "github/python-pubsub/.kokoro/test-samples-against-head.sh"
+    value: "github/python-api-core/.kokoro/test-samples-against-head.sh"
 }
diff --git a/.kokoro/samples/python3.9/periodic-head.cfg b/.kokoro/samples/python3.9/periodic-head.cfg
index f9cfcd3..a18c0cf 100644
--- a/.kokoro/samples/python3.9/periodic-head.cfg
+++ b/.kokoro/samples/python3.9/periodic-head.cfg
@@ -7,5 +7,5 @@ env_vars: {
 
 env_vars: {
     key: "TRAMPOLINE_BUILD_FILE"
-    value: "github/python-pubsub/.kokoro/test-samples-against-head.sh"
+    value: "github/python-api-core/.kokoro/test-samples-against-head.sh"
 }
diff --git a/CHANGELOG.md b/CHANGELOG.md
index 14c16b4..e00e424 100644
--- a/CHANGELOG.md
+++ b/CHANGELOG.md
@@ -4,6 +4,18 @@
 
 [1]: https://pypi.org/project/google-api-core/#history
 
+### [1.31.1](https://www.github.com/googleapis/python-api-core/compare/v1.31.0...v1.31.1) (2021-07-26)
+
+
+### Bug Fixes
+
+* add 'requests.exceptions.ChunkedEncodingError' to retryable exceptions ([#237](https://www.github.com/googleapis/python-api-core/issues/237)) ([5e540f2](https://www.github.com/googleapis/python-api-core/commit/5e540f28493cc3e13260458a8d1c6a1abb2ed313))
+
+
+### Documentation
+
+* add Samples section to CONTRIBUTING.rst ([#229](https://www.github.com/googleapis/python-api-core/issues/229)) ([a12c051](https://www.github.com/googleapis/python-api-core/commit/a12c0516c42918b05415835029717814353b883b))
+
 ## [1.31.0](https://www.github.com/googleapis/python-api-core/compare/v1.30.0...v1.31.0) (2021-07-07)
 
 
diff --git a/CONTRIBUTING.rst b/CONTRIBUTING.rst
index 07c9a61..c7860ad 100644
--- a/CONTRIBUTING.rst
+++ b/CONTRIBUTING.rst
@@ -177,6 +177,30 @@ Build the docs via:
 
    $ nox -s docs
 
+*************************
+Samples and code snippets
+*************************
+
+Code samples and snippets live in the `samples/` catalogue. Feel free to
+provide more examples, but make sure to write tests for those examples.
+Each folder containing example code requires its own `noxfile.py` script
+which automates testing. If you decide to create a new folder, you can
+base it on the `samples/snippets` folder (providing `noxfile.py` and
+the requirements files).
+
+The tests will run against a real Google Cloud Project, so you should
+configure them just like the System Tests.
+
+- To run sample tests, you can execute::
+
+   # Run all tests in a folder
+   $ cd samples/snippets
+   $ nox -s py-3.8
+
+   # Run a single sample test
+   $ cd samples/snippets
+   $ nox -s py-3.8 -- -k <name of test>
+
 ********************************************
 Note About ``README`` as it pertains to PyPI
 ********************************************
diff --git a/google/api_core/protobuf_helpers.py b/google/api_core/protobuf_helpers.py
index 365ef25..8aff79a 100644
--- a/google/api_core/protobuf_helpers.py
+++ b/google/api_core/protobuf_helpers.py
@@ -357,6 +357,13 @@ def _field_mask_helper(original, modified, current=""):
 
 
 def _get_path(current, name):
+    # gapic-generator-python appends underscores to field names
+    # that collide with python keywords.
+    # `_` is stripped away as it is not possible to
+    # natively define a field with a trailing underscore in protobuf.
+    # APIs will reject field masks if fields have trailing underscores.
+    # See https://github.com/googleapis/python-api-core/issues/227
+    name = name.rstrip("_")
     if not current:
         return name
     return "%s.%s" % (current, name)
diff --git a/google/api_core/retry.py b/google/api_core/retry.py
index f0f23bc..8496793 100644
--- a/google/api_core/retry.py
+++ b/google/api_core/retry.py
@@ -104,6 +104,7 @@ if_transient_error = if_exception_type(
     exceptions.TooManyRequests,
     exceptions.ServiceUnavailable,
     requests.exceptions.ConnectionError,
+    requests.exceptions.ChunkedEncodingError,
     auth_exceptions.TransportError,
 )
 """A predicate that checks if an exception is a transient API error.
diff --git a/google/api_core/version.py b/google/api_core/version.py
index c90c1c0..5fd0b13 100644
--- a/google/api_core/version.py
+++ b/google/api_core/version.py
@@ -12,4 +12,4 @@
 # See the License for the specific language governing permissions and
 # limitations under the License.
 
-__version__ = "1.31.0"
+__version__ = "1.31.1"
diff --git a/noxfile.py b/noxfile.py
index 2560992..2f11137 100644
--- a/noxfile.py
+++ b/noxfile.py
@@ -98,9 +98,10 @@ def default(session):
     ]
     pytest_args.extend(session.posargs)
 
-    # Inject AsyncIO content, if version >= 3.6.
+    # Inject AsyncIO content and proto-plus, if version >= 3.6.
+    # proto-plus is needed for a field mask test in test_protobuf_helpers.py
     if _greater_or_equal_than_36(session.python):
-        session.install("asyncmock", "pytest-asyncio")
+        session.install("asyncmock", "pytest-asyncio", "proto-plus")
 
         pytest_args.append("--cov=tests.asyncio")
         pytest_args.append(os.path.join("tests", "asyncio"))


Test patch:
diff --git a/tests/unit/test_protobuf_helpers.py b/tests/unit/test_protobuf_helpers.py
index db97238..3df45df 100644
--- a/tests/unit/test_protobuf_helpers.py
+++ b/tests/unit/test_protobuf_helpers.py
@@ -12,6 +12,8 @@
 # See the License for the specific language governing permissions and
 # limitations under the License.
 
+import sys
+
 import pytest
 
 from google.api import http_pb2
@@ -472,3 +474,45 @@ def test_field_mask_different_level_diffs():
         "alpha",
         "red",
     ]
+
+
+@pytest.mark.skipif(
+    sys.version_info.major == 2,
+    reason="Field names with trailing underscores can only be created"
+    "through proto-plus, which is Python 3 only.",
+)
+def test_field_mask_ignore_trailing_underscore():
+    import proto
+
+    class Foo(proto.Message):
+        type_ = proto.Field(proto.STRING, number=1)
+        input_config = proto.Field(proto.STRING, number=2)
+
+    modified = Foo(type_="bar", input_config="baz")
+
+    assert sorted(protobuf_helpers.field_mask(None, Foo.pb(modified)).paths) == [
+        "input_config",
+        "type",
+    ]
+
+
+@pytest.mark.skipif(
+    sys.version_info.major == 2,
+    reason="Field names with trailing underscores can only be created"
+    "through proto-plus, which is Python 3 only.",
+)
+def test_field_mask_ignore_trailing_underscore_with_nesting():
+    import proto
+
+    class Bar(proto.Message):
+        class Baz(proto.Message):
+            input_config = proto.Field(proto.STRING, number=1)
+
+        type_ = proto.Field(Baz, number=1)
+
+    modified = Bar()
+    modified.type_.input_config = "foo"
+
+    assert sorted(protobuf_helpers.field_mask(None, Bar.pb(modified)).paths) == [
+        "type.input_config",
+    ]
diff --git a/tests/unit/test_retry.py b/tests/unit/test_retry.py
index f24c82b..199ca55 100644
--- a/tests/unit/test_retry.py
+++ b/tests/unit/test_retry.py
@@ -45,6 +45,7 @@ def test_if_transient_error():
     assert retry.if_transient_error(exceptions.TooManyRequests(""))
     assert retry.if_transient_error(exceptions.ServiceUnavailable(""))
     assert retry.if_transient_error(requests.exceptions.ConnectionError(""))
+    assert retry.if_transient_error(requests.exceptions.ChunkedEncodingError(""))
     assert retry.if_transient_error(auth_exceptions.TransportError(""))
     assert not retry.if_transient_error(exceptions.InvalidArgument(""))