let build() skip docstring generation - #2822
rootkiller6788 wants to merge 2 commits into
Conversation
Expanding every method's request/response schema into a docstring is the expensive part of building a service. For sheets.v4, calling service.spreadsheets() peaks at ~57 MB just for that, and it's paid again for every service built. Add a generate_docstrings flag to build() and build_from_document() so callers that never read the docstrings can opt out. It defaults to True, so nothing changes for existing code.
Covers both build() and build_from_document(), and checks the flag reaches lazily built nested resources (plus.activities().list). Also updates test_pickle, which pins the exact set of Resource attributes.
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
There was a problem hiding this comment.
Code Review
This pull request introduces a generate_docstrings parameter (defaulting to True) to the service building functions in googleapiclient/discovery.py. When set to False, it skips the resource-intensive process of generating method docstrings, which helps reduce memory usage and startup time in environments where docstrings are not needed. The review feedback points out that the newly added test test_build_without_docstrings_still_builds_requests only asserts that docstrings are None without actually verifying that requests can still be successfully built. A code suggestion is provided to explicitly build and assert both standard and media requests.
| def test_build_without_docstrings_still_builds_requests(self): | ||
| http = HttpMock(datafile("zoo.json"), {"status": "200"}) | ||
| zoo = build( | ||
| "zoo", "v1", http=http, static_discovery=False, generate_docstrings=False | ||
| ) | ||
| self.assertIsNone(zoo.animals().get.__doc__) | ||
| # The generated _media variants go through the same path. | ||
| self.assertIsNone(zoo.animals().get_media.__doc__) |
There was a problem hiding this comment.
The test test_build_without_docstrings_still_builds_requests is intended to verify that building a service without docstrings still allows requests to be built successfully. However, the test currently only asserts that the docstrings are None and does not actually attempt to build any requests. To ensure that request building is indeed functional and not broken by the lack of docstrings, we should explicitly build both a standard request and a media request and assert that they are created successfully.
| def test_build_without_docstrings_still_builds_requests(self): | |
| http = HttpMock(datafile("zoo.json"), {"status": "200"}) | |
| zoo = build( | |
| "zoo", "v1", http=http, static_discovery=False, generate_docstrings=False | |
| ) | |
| self.assertIsNone(zoo.animals().get.__doc__) | |
| # The generated _media variants go through the same path. | |
| self.assertIsNone(zoo.animals().get_media.__doc__) | |
| def test_build_without_docstrings_still_builds_requests(self): | |
| http = HttpMock(datafile("zoo.json"), {"status": "200"}) | |
| zoo = build( | |
| "zoo", "v1", http=http, static_discovery=False, generate_docstrings=False | |
| ) | |
| self.assertIsNone(zoo.animals().get.__doc__) | |
| # The generated _media variants go through the same path. | |
| self.assertIsNone(zoo.animals().get_media.__doc__) | |
| req = zoo.animals().get(name="Lion") | |
| self.assertIsNotNone(req) | |
| media_req = zoo.animals().get_media(name="Lion") | |
| self.assertIsNotNone(media_req) |
This branch has not been deployed
1 participant
Fixes #2779.
Building a service expands every method's request and response schema into a docstring, and that is basically the whole cost of building one. I measured sheets.v4 on this branch: calling service.spreadsheets() peaks at 57 MB and takes 0.29s, and 9.7M characters of docstring come out of it. In a server that builds a service per request that adds up fast, and nobody reads those docstrings there.
So I added generate_docstrings to build() and build_from_document(), defaulting to True so nothing changes for existing code. When it's False createMethod returns before building the docs, and the flag is remembered on the Resource so lazily built sub-resources inherit it. I also threaded it through the _media variants, which go through the same helper.
With it off the same call is ~0 MB / 0.00s and a request built from the service is byte for byte the same, so the only thing lost is the docstrings. Tests cover build() and build_from_document(), plus that the flag reaches nested resources that are only created on demand. I had to add _generate_docstrings to the attribute list in test_pickle.