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: move to using insecure grpc channels with emulator #402

Merged
merged 13 commits into from Jul 22, 2021

Conversation

crwilcox
Copy link
Contributor

Related to #359

On windows, configuring a secure channel with credentials will not connect.

@crwilcox crwilcox added the do not merge Indicates a pull request not ready for merge, due to either quality or timing. label Jul 21, 2021
@crwilcox crwilcox requested a review from a team July 21, 2021 00:43
@crwilcox crwilcox requested a review from a team as a code owner July 21, 2021 00:43
@product-auto-label product-auto-label bot added the api: firestore Issues related to the googleapis/python-firestore API. label Jul 21, 2021
@google-cla google-cla bot added the cla: yes This human has signed the Contributor License Agreement. label Jul 21, 2021
return grpc.aio.secure_channel(
self._emulator_host, self._local_composite_credentials()
)
return grpc.aio.insecure_channel(self._emulator_host)
Copy link
Contributor

Choose a reason for hiding this comment

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

Can we possibly have our cake and eat it too by introducing another environment variable for these custom credentials?

@crwilcox
Copy link
Contributor Author

@paulharter FYI. This should still support your use case, but as there was no test added to demonstrate your complete use, I am unsure. Reach out if this doesn't work and we can find a way to support both use case :)

@crwilcox crwilcox removed the do not merge Indicates a pull request not ready for merge, due to either quality or timing. label Jul 21, 2021
@crwilcox crwilcox force-pushed the use-insecure-channels-emulator branch from 8064453 to b2af8b2 Compare July 21, 2021 22:51
Copy link
Contributor

@craiglabenz craiglabenz left a comment

Choose a reason for hiding this comment

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

LGTM

google/cloud/firestore_v1/base_client.py Outdated Show resolved Hide resolved
google/cloud/firestore_v1/base_client.py Show resolved Hide resolved
tests/unit/v1/test_base_client.py Show resolved Hide resolved
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
api: firestore Issues related to the googleapis/python-firestore API. cla: yes This human has signed the Contributor License Agreement.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants