Skip to content

Ensure gRPC server readiness before proceeding to prevent test failures#5938

Merged
nabenabe0928 merged 3 commits intooptuna:masterfrom
sawa3030:fix_grpc_server_test
Jan 28, 2025
Merged

Ensure gRPC server readiness before proceeding to prevent test failures#5938
nabenabe0928 merged 3 commits intooptuna:masterfrom
sawa3030:fix_grpc_server_test

Conversation

@sawa3030
Copy link
Copy Markdown
Collaborator

@sawa3030 sawa3030 commented Jan 23, 2025

Motivation

ref: Fix StorageSupplier("grpc") to make unit tests robust. #5925
The issue occurs when the proxy begins communicating with the server before the server has fully started. By ensuring the server is ready before proceeding, this change aims to make the unit tests more robust.

Description of the changes

  • Implement loop to confirm that the gRPC server is ready before continuing execution.

Comment on lines +104 to +109
while True:
try:
proxy.get_all_studies()
break
except grpc.RpcError:
continue
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Could you add a time.sleep() to avoid a busy loop?

Suggested change
while True:
try:
proxy.get_all_studies()
break
except grpc.RpcError:
continue
while True:
try:
proxy.get_all_studies()
break
except grpc.RpcError:
time.sleep(1)
continue

@c-bata c-bata self-assigned this Jan 23, 2025
@c-bata c-bata added the CI Continuous integration. label Jan 23, 2025
@c-bata
Copy link
Copy Markdown
Member

c-bata commented Jan 24, 2025

@nabenabe0928 Could you review this PR?

Copy link
Copy Markdown
Member

@c-bata c-bata left a comment

Choose a reason for hiding this comment

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

LGTM.

@c-bata c-bata removed their assignment Jan 24, 2025
Copy link
Copy Markdown
Contributor

@nabenabe0928 nabenabe0928 left a comment

Choose a reason for hiding this comment

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

LGTM!

@nabenabe0928 nabenabe0928 merged commit fc22ea7 into optuna:master Jan 28, 2025
@nabenabe0928 nabenabe0928 removed their assignment Jan 30, 2025
@nabenabe0928 nabenabe0928 added this to the v4.3.0 milestone Jan 30, 2025
@sawa3030 sawa3030 deleted the fix_grpc_server_test branch November 14, 2025 07:48
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

CI Continuous integration.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants