Skip to content

chore: regenerate geminidataanalytics#17303

Merged
parthea merged 1 commit into
googleapis:mainfrom
suztomo:regenerate_and_release_geminidataanalytics
May 28, 2026
Merged

chore: regenerate geminidataanalytics#17303
parthea merged 1 commit into
googleapis:mainfrom
suztomo:regenerate_and_release_geminidataanalytics

Conversation

@suztomo
Copy link
Copy Markdown
Member

@suztomo suztomo commented May 28, 2026

$ docker run -u $(id -u):$(id -g) -v .:/repo -v ~/.cache:/.cache -w /repo docker.io/library/librarian-python:${V} generate -v google-cloud-geminidataanalytics where V is v0.15.1-0.20260528141105-567c9bf1faa7.

@suztomo suztomo requested a review from a team as a code owner May 28, 2026 21:19
@suztomo suztomo force-pushed the regenerate_and_release_geminidataanalytics branch from 2a9c0c0 to ed36086 Compare May 28, 2026 21:20
Copy link
Copy Markdown
Contributor

@gemini-code-assist gemini-code-assist Bot left a comment

Choose a reason for hiding this comment

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

Code Review

This pull request updates the minimum grpcio dependency version to 1.59.0 in setup.py and constraints, adjusts noxfile.py to conditionally locate lower-bound constraints and remove system test dependency installations, and refactors the test suite across multiple API versions. Specifically, it introduces an autouse set_event_loop fixture for asyncio tests and consolidates dictionary-based request tests using pytest.mark.parametrize. The reviewer feedback recommends optimizing the set_event_loop fixture to only execute for asyncio-marked tests, avoiding unnecessary setup and teardown overhead for the synchronous tests that make up the majority of the test suite.

Comment on lines +132 to +144
@pytest.fixture(autouse=True)
def set_event_loop():
try:
asyncio.get_running_loop()
yield
except RuntimeError:
loop = asyncio.new_event_loop()
asyncio.set_event_loop(loop)
try:
yield
finally:
loop.close()
asyncio.set_event_loop(None)
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

medium

The set_event_loop fixture is marked as autouse=True, meaning it runs for every single test in this module. Since the vast majority of the tests in this file are synchronous, creating, setting, closing, and clearing an event loop for each synchronous test introduces unnecessary performance overhead. We can optimize this by checking if the test is an asyncio test and yielding early for synchronous tests.

Suggested change
@pytest.fixture(autouse=True)
def set_event_loop():
try:
asyncio.get_running_loop()
yield
except RuntimeError:
loop = asyncio.new_event_loop()
asyncio.set_event_loop(loop)
try:
yield
finally:
loop.close()
asyncio.set_event_loop(None)
@pytest.fixture(autouse=True)
def set_event_loop(request):
if "asyncio" not in request.keywords:
yield
return
try:
asyncio.get_running_loop()
yield
except RuntimeError:
loop = asyncio.new_event_loop()
asyncio.set_event_loop(loop)
try:
yield
finally:
loop.close()
asyncio.set_event_loop(None)

Comment on lines +123 to +135
@pytest.fixture(autouse=True)
def set_event_loop():
try:
asyncio.get_running_loop()
yield
except RuntimeError:
loop = asyncio.new_event_loop()
asyncio.set_event_loop(loop)
try:
yield
finally:
loop.close()
asyncio.set_event_loop(None)
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

medium

The set_event_loop fixture is marked as autouse=True, meaning it runs for every single test in this module. Since the vast majority of the tests in this file are synchronous, creating, setting, closing, and clearing an event loop for each synchronous test introduces unnecessary performance overhead. We can optimize this by checking if the test is an asyncio test and yielding early for synchronous tests.

Suggested change
@pytest.fixture(autouse=True)
def set_event_loop():
try:
asyncio.get_running_loop()
yield
except RuntimeError:
loop = asyncio.new_event_loop()
asyncio.set_event_loop(loop)
try:
yield
finally:
loop.close()
asyncio.set_event_loop(None)
@pytest.fixture(autouse=True)
def set_event_loop(request):
if "asyncio" not in request.keywords:
yield
return
try:
asyncio.get_running_loop()
yield
except RuntimeError:
loop = asyncio.new_event_loop()
asyncio.set_event_loop(loop)
try:
yield
finally:
loop.close()
asyncio.set_event_loop(None)

Comment on lines +134 to +146
@pytest.fixture(autouse=True)
def set_event_loop():
try:
asyncio.get_running_loop()
yield
except RuntimeError:
loop = asyncio.new_event_loop()
asyncio.set_event_loop(loop)
try:
yield
finally:
loop.close()
asyncio.set_event_loop(None)
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

medium

The set_event_loop fixture is marked as autouse=True, meaning it runs for every single test in this module. Since the vast majority of the tests in this file are synchronous, creating, setting, closing, and clearing an event loop for each synchronous test introduces unnecessary performance overhead. We can optimize this by checking if the test is an asyncio test and yielding early for synchronous tests.

Suggested change
@pytest.fixture(autouse=True)
def set_event_loop():
try:
asyncio.get_running_loop()
yield
except RuntimeError:
loop = asyncio.new_event_loop()
asyncio.set_event_loop(loop)
try:
yield
finally:
loop.close()
asyncio.set_event_loop(None)
@pytest.fixture(autouse=True)
def set_event_loop(request):
if "asyncio" not in request.keywords:
yield
return
try:
asyncio.get_running_loop()
yield
except RuntimeError:
loop = asyncio.new_event_loop()
asyncio.set_event_loop(loop)
try:
yield
finally:
loop.close()
asyncio.set_event_loop(None)

Comment on lines +126 to +138
@pytest.fixture(autouse=True)
def set_event_loop():
try:
asyncio.get_running_loop()
yield
except RuntimeError:
loop = asyncio.new_event_loop()
asyncio.set_event_loop(loop)
try:
yield
finally:
loop.close()
asyncio.set_event_loop(None)
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

medium

The set_event_loop fixture is marked as autouse=True, meaning it runs for every single test in this module. Since the vast majority of the tests in this file are synchronous, creating, setting, closing, and clearing an event loop for each synchronous test introduces unnecessary performance overhead. We can optimize this by checking if the test is an asyncio test and yielding early for synchronous tests.

Suggested change
@pytest.fixture(autouse=True)
def set_event_loop():
try:
asyncio.get_running_loop()
yield
except RuntimeError:
loop = asyncio.new_event_loop()
asyncio.set_event_loop(loop)
try:
yield
finally:
loop.close()
asyncio.set_event_loop(None)
@pytest.fixture(autouse=True)
def set_event_loop(request):
if "asyncio" not in request.keywords:
yield
return
try:
asyncio.get_running_loop()
yield
except RuntimeError:
loop = asyncio.new_event_loop()
asyncio.set_event_loop(loop)
try:
yield
finally:
loop.close()
asyncio.set_event_loop(None)

Comment on lines +134 to +146
@pytest.fixture(autouse=True)
def set_event_loop():
try:
asyncio.get_running_loop()
yield
except RuntimeError:
loop = asyncio.new_event_loop()
asyncio.set_event_loop(loop)
try:
yield
finally:
loop.close()
asyncio.set_event_loop(None)
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

medium

The set_event_loop fixture is marked as autouse=True, meaning it runs for every single test in this module. Since the vast majority of the tests in this file are synchronous, creating, setting, closing, and clearing an event loop for each synchronous test introduces unnecessary performance overhead. We can optimize this by checking if the test is an asyncio test and yielding early for synchronous tests.

Suggested change
@pytest.fixture(autouse=True)
def set_event_loop():
try:
asyncio.get_running_loop()
yield
except RuntimeError:
loop = asyncio.new_event_loop()
asyncio.set_event_loop(loop)
try:
yield
finally:
loop.close()
asyncio.set_event_loop(None)
@pytest.fixture(autouse=True)
def set_event_loop(request):
if "asyncio" not in request.keywords:
yield
return
try:
asyncio.get_running_loop()
yield
except RuntimeError:
loop = asyncio.new_event_loop()
asyncio.set_event_loop(loop)
try:
yield
finally:
loop.close()
asyncio.set_event_loop(None)

Comment on lines +126 to +138
@pytest.fixture(autouse=True)
def set_event_loop():
try:
asyncio.get_running_loop()
yield
except RuntimeError:
loop = asyncio.new_event_loop()
asyncio.set_event_loop(loop)
try:
yield
finally:
loop.close()
asyncio.set_event_loop(None)
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

medium

The set_event_loop fixture is marked as autouse=True, meaning it runs for every single test in this module. Since the vast majority of the tests in this file are synchronous, creating, setting, closing, and clearing an event loop for each synchronous test introduces unnecessary performance overhead. We can optimize this by checking if the test is an asyncio test and yielding early for synchronous tests.

Suggested change
@pytest.fixture(autouse=True)
def set_event_loop():
try:
asyncio.get_running_loop()
yield
except RuntimeError:
loop = asyncio.new_event_loop()
asyncio.set_event_loop(loop)
try:
yield
finally:
loop.close()
asyncio.set_event_loop(None)
@pytest.fixture(autouse=True)
def set_event_loop(request):
if "asyncio" not in request.keywords:
yield
return
try:
asyncio.get_running_loop()
yield
except RuntimeError:
loop = asyncio.new_event_loop()
asyncio.set_event_loop(loop)
try:
yield
finally:
loop.close()
asyncio.set_event_loop(None)

@parthea parthea merged commit 569c823 into googleapis:main May 28, 2026
31 checks passed
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

Successfully merging this pull request may close these issues.

2 participants