Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

fix: disambiguate field headers whose names are reserved python words #1178

Merged
merged 5 commits into from Feb 12, 2022

Conversation

parthea
Copy link
Contributor

@parthea parthea commented Feb 2, 2022

Fixes #1177

@parthea
Copy link
Contributor Author

parthea commented Feb 2, 2022

The integration test is failing with

FileNotFoundError: [Errno 2] No such file or directory: '/github/home/.cache/bazel/_bazel_root/e9fc40f6defbd5492f3d1bc9251d68a4/sandbox/processwrapper-sandbox/1439/execroot/gapic_generator_python/bazel-out/host/bin/gapic_plugin.runfiles/gapic_generator_python_pip_deps/pypi__setuptools/pkg_resources/_vendor/jaraco/text/Lorem ipsum.txt'

It may be related to pypa/setuptools#3071 and pypa/setuptools#3061

The stack trace is similar

 from pkg_resources.extern.jaraco.text import (
  File "/usr/local/google/home/partheniou/.cache/bazel/_bazel_partheniou/9063cec94a6a99d93035235cd3e5a885/sandbox/linux-sandbox/1405/execroot/com_google_googleapis/bazel-out/host/bin/external/gapic_generator_python/gapic_plugin.runfiles/gapic_generator_python_pip_deps/pypi__setuptools/pkg_resources/_vendor/jaraco/text/__init__.py", line 227, in <module>
    lorem_ipsum: str = files(__name__).joinpath('Lorem ipsum.txt').read_text()
  File "/usr/lib/python3.9/pathlib.py", line 1266, in read_text
    with self.open(mode='r', encoding=encoding, errors=errors) as f:
  File "/usr/lib/python3.9/pathlib.py", line 1252, in open
    return io.open(self, mode, buffering, encoding, errors, newline,
  File "/usr/lib/python3.9/pathlib.py", line 1120, in _opener
    return self._accessor.open(self, flags, mode)
FileNotFoundError: [Errno 2] No such file or directory: '/usr/local/google/home/partheniou/.cache/bazel/_bazel_partheniou/9063cec94a6a99d93035235cd3e5a885/sandbox/linux-sandbox/1405/execroot/com_google_googleapis/bazel-out/host/bin/external/gapic_generator_python/gapic_plugin.runfiles/gapic_generator_python_pip_deps/pypi__setuptools/pkg_resources/_vendor/jaraco/text/Lorem ipsum.txt'

@parthea
Copy link
Contributor Author

parthea commented Feb 2, 2022

This is ready for review. I've confirmed that the unit tests pass locally in the generated client for datastream v1 with the changes from this PR.

partheniou@partheniou-vm-1:~/git/python-datastream$ nox -s unit-3.8
nox > Running session unit-3.8
nox > Creating virtual environment (virtualenv) using python3.8 in .nox/unit-3-8
nox > python -m pip install mock asyncmock pytest pytest-cov pytest-asyncio -c /usr/local/google/home/partheniou/git/python-datastream/testing/constraints-3.8.txt
nox > python -m pip install -e . -c /usr/local/google/home/partheniou/git/python-datastream/testing/constraints-3.8.txt
nox > py.test --quiet --junitxml=unit_3.8_sponge_log.xml --cov=google --cov=tests/unit --cov-append --cov-config=.coveragerc --cov-report= --cov-fail-under=0 tests/unit
....................................................................................................................................................................................... [ 26%]
....................................................................................................................................................................................... [ 53%]
....................................................................................................................................................................................... [ 80%]
.......................................................................................................................................                                                 [100%]
====================================================================================== warnings summary =======================================================================================

Copy link
Contributor

@software-dov software-dov left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks good, just a personal preference nit.

gapic/schema/wrappers.py Outdated Show resolved Hide resolved
Copy link
Contributor

@vam-google vam-google left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

@parthea parthea added the do not merge Indicates a pull request not ready for merge, due to either quality or timing. label Feb 3, 2022
@software-dov
Copy link
Contributor

Just curious, why mark this PR as Do Not Merge?

@parthea
Copy link
Contributor Author

parthea commented Feb 3, 2022

@software-dov I added do not merge because I still need to apply the changes from the review feedback but I haven't got around to it.

@parthea parthea added automerge Merge the pull request once unit tests and other checks pass. and removed do not merge Indicates a pull request not ready for merge, due to either quality or timing. labels Feb 12, 2022
@parthea parthea merged commit 98aa690 into main Feb 12, 2022
@parthea parthea deleted the disambiguate-field-headers branch February 12, 2022 17:10
@gcf-merge-on-green gcf-merge-on-green bot removed the automerge Merge the pull request once unit tests and other checks pass. label Feb 12, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Naming collision with variable for field headers
3 participants