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

Inaccurate Dosctrings in Modules: google/pubsub_v1/services/subscriber #1020

Closed
gentris opened this issue Nov 6, 2023 · 4 comments · May be fixed by #1021
Closed

Inaccurate Dosctrings in Modules: google/pubsub_v1/services/subscriber #1020

gentris opened this issue Nov 6, 2023 · 4 comments · May be fixed by #1021
Assignees
Labels
api: pubsub Issues related to the googleapis/python-pubsub API.

Comments

@gentris
Copy link

gentris commented Nov 6, 2023

Summary

Several functions within the following modules are affected by misleading docstrings, which may result in confusion for users seeking to understand the behavior of these functions.

The affected modules include:

  • google/pubsub_v1/services/subscriber/async_client.py
  • google/pubsub_v1/services/subscriber/client.py
  • google/pubsub_v1/services/subscriber/transports/grpc.py
  • google/pubsub_v1/services/subscriber/transports/grpc_asyncio.py

Details

The issue primarily revolves around the incorrect usage of the term "returns" in the docstrings for certain functions, specifically when describing their behavior during exceptional circumstances. In these cases, the functions are expected to raise exceptions such as ALREADY_EXISTS and NOT_FOUND instead of returning these exceptions.

This inconsistency between the docstring descriptions and the actual behavior of the functions can lead to misunderstandings, potentially impacting the usability and reliability of the codebase.

Proposed Solution

The recommended solution is to update the docstrings in the affected modules to accurately reflect the behavior of these functions. This correction will enhance the clarity and correctness of the documentation, making it easier for users to comprehend the expected behavior of the functions during edge cases.

@pradn
Copy link
Contributor

pradn commented Nov 6, 2023

Hey @gentris, good catch. An example of such a mistake is here. These files are auto-generated, so I'll have to look into where I can fix these.

As an aside, did the docstrings come up via your IDE or were you looking at the files directly?

@gentris
Copy link
Author

gentris commented Nov 6, 2023

Hey @pradn, I read the documentation (docstrings) directly in the file. But even now, using the PyCharm IDE for example, it's displayed as returns instead of raises. See the attached screenshot.
Screenshot 2023-11-06 at 3 19 22 PM

@pradn
Copy link
Contributor

pradn commented Nov 6, 2023

Okay, I see. That's helpful.

Thank you for taking the time to write up #1021, but we can't merge it because it's modifying auto-generated code. Your changes will be overridden in the next generation step. We would need to modify the CPS API definition proto file upsteam. But first, we're going to discuss internally and see.

@pradn
Copy link
Contributor

pradn commented Nov 6, 2023

I know this is a problem, but we're going to hold off on making changes for now. There's quite a lot of similar wording for CPS APIs and also for other GCP products. We'd want to consider a wider solution, perhaps one that tailors wording per library. Thank you for filing the issue.

@pradn pradn closed this as not planned Won't fix, can't repro, duplicate, stale Nov 6, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
api: pubsub Issues related to the googleapis/python-pubsub API.
Projects
None yet
2 participants