-
Notifications
You must be signed in to change notification settings - Fork 988
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
Client change google api #2530
Client change google api #2530
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Minor comments. Otherwise LGTM
cirq/google/api/v2/results_test.py
Outdated
from cirq.google.api import v2 | ||
from cirq.google.api.v2 import result_pb2 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Alternately, remove this and change to v2.result_pb2
in the code.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Done.
cirq/google/api/v2/sweeps_test.py
Outdated
@@ -18,7 +18,7 @@ | |||
import sympy | |||
|
|||
import cirq | |||
from cirq.api.google.v2 import run_context_pb2 | |||
from cirq.google.api.v2 import run_context_pb2 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Could remove this and refer to v2.run_context_pb2
below.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Done.
We should be good to switch the client side to the newest type URL (cirq.google.api). All downstream processes should now be able to support both type URLs.
This PR switches cirq to use cirq.google.api protos rather than cirq.api.google protos.