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

feat(pubsub/pstest): support listening on custom host #11586

Open
wants to merge 7 commits into
base: main
Choose a base branch
from

Conversation

larryanz
Copy link

@larryanz larryanz commented Feb 12, 2025

Regarding #11569, this PR adds support of pstest test server to listening on custom hosts rather than only localhost.

@larryanz larryanz requested review from shollyman and a team as code owners February 12, 2025 10:02
Copy link

google-cla bot commented Feb 12, 2025

Thanks for your pull request! It looks like this may be your first contribution to a Google open source project. Before we can look at your pull request, you'll need to sign a Contributor License Agreement (CLA).

View this failed invocation of the CLA check for more information.

For the most up to date status, view the checks section at the bottom of the pull request.

@product-auto-label product-auto-label bot added the api: pubsub Issues related to the Pub/Sub API. label Feb 12, 2025
@larryanz larryanz changed the title pubsub/pstest: support listening on custom host feat(pubsub/pstest): support listening on custom host Feb 12, 2025
@larryanz
Copy link
Author

Hi @hongalex, please help to take a look

Copy link
Member

@hongalex hongalex left a comment

Choose a reason for hiding this comment

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

Looks good overall, but I have some concerns about naming

internal/testutil/server.go Outdated Show resolved Hide resolved
internal/testutil/server_test.go Outdated Show resolved Hide resolved
@larryanz larryanz force-pushed the feat/support-listening-hostname-0000 branch from c24aa0d to a20f447 Compare February 13, 2025 05:17
// at the specified address (host and port). Before starting the server, the
// provided callback is called to allow caller to register additional fakes into
// grpc server.
func NewServerWithCallback(address string, callback func(*grpc.Server), opts ...ServerReactorOption) *Server {
Copy link
Member

Choose a reason for hiding this comment

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

Changing the signature of NewServerWithCallback is a breaking change, and although we do allow for breaking changes within the fake, we should probably avoid it when it is easy to do so. Can you revert this change?

Copy link
Author

Choose a reason for hiding this comment

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

updated, thanks!

@hongalex hongalex added the kokoro:run Add this label to force Kokoro to re-run the tests. label Feb 13, 2025
@kokoro-team kokoro-team removed the kokoro:run Add this label to force Kokoro to re-run the tests. label Feb 13, 2025
@larryanz larryanz force-pushed the feat/support-listening-hostname-0000 branch from 9fee3f3 to ab6fe46 Compare February 14, 2025 03:51
@larryanz
Copy link
Author

hi @hongalex , seems the CI fails, could you please try it again?

@hongalex hongalex enabled auto-merge (squash) February 14, 2025 21:21
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 Pub/Sub API.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants