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

Renaming http argument(s) as _http. #3235

Merged
merged 6 commits into from
Mar 30, 2017

Conversation

dhermes
Copy link
Contributor

@dhermes dhermes commented Mar 29, 2017

Will also shortly be sending a commit to this PR which renames use_gax and _use_grpc.

grep-ing for http and http= was pretty ad-hoc, so I'm not sure if there are lingering things needing to be changed (e.g. docs which refer to http instead of _http)

Note in the commit about use_gax the following commands were used:

$ git grep -l use_gax | xargs sed -i s/use_gax/_use_grpc/g
$ git grep -l __use_grpc | xargs sed -i s/__use_grpc/_use_grpc/g
$ git grep -l USE_GAX | xargs sed -i s/USE_GAX/USE_GRPC/g
$ git grep -l HAVE_GAX | xargs sed -i s/HAVE_GAX/HAVE_GRPC/g
$ git grep -l DISABLE_GAX | xargs sed -i s/DISABLE_GAX/DISABLE_GRPC/g

@googlebot googlebot added the cla: yes This human has signed the Contributor License Agreement. label Mar 29, 2017
dhermes and others added 2 commits March 29, 2017 15:50
Done via several commands from the CLI:

$ git grep -l use_gax | xargs sed -i s/use_gax/_use_grpc/g
$ git grep -l __use_grpc | xargs sed -i s/__use_grpc/_use_grpc/g
$ git grep -l USE_GAX | xargs sed -i s/USE_GAX/USE_GRPC/g
$ git grep -l HAVE_GAX | xargs sed -i s/HAVE_GAX/HAVE_GRPC/g
$ git grep -l DISABLE_GAX | xargs sed -i s/DISABLE_GAX/DISABLE_GRPC/g

as well as some manual edits for indent / line length (only did
this around `:param _use_grpc:`).
@dhermes
Copy link
Contributor Author

dhermes commented Mar 30, 2017

@lukesneeringer 👍 on your commit

@dhermes
Copy link
Contributor Author

dhermes commented Mar 30, 2017

@lukesneeringer We all good here?

@lukesneeringer lukesneeringer merged commit 0f12813 into googleapis:master Mar 30, 2017
dhermes added a commit to dhermes/google-cloud-python that referenced this pull request Mar 30, 2017
Issue caused by two unrelated PRs merging in close
proximity: googleapis#3235 and googleapis#3230.
lukesneeringer pushed a commit that referenced this pull request Mar 30, 2017
Issue caused by two unrelated PRs merging in close
proximity: #3235 and #3230.
@dhermes dhermes deleted the make-http-private branch March 30, 2017 16:53
@surjikal
Copy link

surjikal commented Mar 31, 2017

@dhermes I just started getting errors that might have to do with this change. It's happening in my dataproc jobs, but I was able to reproduce the error in a fresh VM instance:

$ sudo easy_install pip
$ sudo pip install google-cloud google-cloud-storage
$ python -c "from google.cloud import storage; storage.Client(project='<project>')"

Traceback (most recent call last):
  File "<string>", line 1, in <module>
  File "/usr/local/lib/python2.7/dist-packages/google/cloud/storage/client.py", line 59, in 
__init__
    _http=_http)
TypeError: __init__() got an unexpected keyword argument '_http'

I'm going to poke around the code in the VM to see what's up.

@dhermes
Copy link
Contributor Author

dhermes commented Mar 31, 2017

I'd guess you have versions of google-cloud-core and google-cloud-storage that conflict with one another.

@dhermes
Copy link
Contributor Author

dhermes commented Mar 31, 2017

Thanks for letting us know though!

@surjikal
Copy link

surjikal commented Mar 31, 2017

I ran pip install google-cloud-core and now all is good. Thanks!

@supertom
Copy link
Contributor

@lukesneeringer @dhermes Is there background as to why the http object was made private?

I haven't looked at this PR in detail yet, but has this change been made across all current clients? Future clients will adhere to the same contract?

@dhermes
Copy link
Contributor Author

dhermes commented Apr 24, 2017

@supertom Yes this was made across all clients. It was done because we will be changing the underlying HTTP transport implementation, so making it part of the public interface was a bad idea (i.e. since we'd be breaking that interface).

richkadel pushed a commit to richkadel/google-cloud-python that referenced this pull request May 6, 2017
richkadel pushed a commit to richkadel/google-cloud-python that referenced this pull request May 6, 2017
Issue caused by two unrelated PRs merging in close
proximity: googleapis#3235 and googleapis#3230.
atulep pushed a commit that referenced this pull request Apr 3, 2023
atulep pushed a commit that referenced this pull request Apr 18, 2023
parthea pushed a commit that referenced this pull request Jun 4, 2023
parthea pushed a commit that referenced this pull request Jul 6, 2023
parthea pushed a commit that referenced this pull request Sep 22, 2023
…-samples#3235)

* video: fix flaky beta tests

* fix failing test with new video file

* add local file tests

* update test

Co-authored-by: Takashi Matsuo <tmatsuo@google.com>
parthea pushed a commit that referenced this pull request Oct 21, 2023
parthea pushed a commit that referenced this pull request Oct 21, 2023
parthea pushed a commit that referenced this pull request Oct 21, 2023
parthea pushed a commit that referenced this pull request Oct 22, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
api: core cla: yes This human has signed the Contributor License Agreement.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants