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

issue: make clients accept strings and typed objects for resource names #48

Closed
anguillanneuf opened this issue Oct 16, 2020 · 4 comments
Assignees

Comments

@anguillanneuf
Copy link
Collaborator

Currently the admin client expects strings

topic_path = str(TopicPath(project_number, location, topic_id))
response = client.get_topic(topic_path)

while the publisher and subscriber clients expect typed object.

topic_path = TopicPath(project_number, location, topic_id)
with make_publisher(topic_path) as publisher_client:

Ideally we shall accept both formats.

@dpcollins-google
Copy link
Collaborator

Ideally we shall accept both formats.

I don't agree with this, the wrapper objects cleanly isolate parsing vs client errors, and have a "parse" method to get this from the underlying string. I'm going to change the admin layer to just accept the wrappers for consistency, is this an acceptable end-state? It would mean you'd call:

response = client.get_topic(TopicPath(project_number, location, topic_id))
or
response = client.get_topic(TopicPath.parse("projects/xxx/locations/xxx/topics/xxx"))

@dpcollins-google
Copy link
Collaborator

I just checked this: The following code will not type check:

 topic_path = str(TopicPath(project_number, location, topic_id)) 
  
 response = client.get_topic(topic_path) 

If you ran it through pytype, it would fail. The current state is the proposed end state, it just happens to work when you pass it a string path.

@anguillanneuf
Copy link
Collaborator Author

Okay, you can just accept the wrappers for consistency.

@dpcollins-google
Copy link
Collaborator

Then this can be closed as they already only accept the wrappers.

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

No branches or pull requests

2 participants