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

Fix data connect emulator listen address. #7211

Merged
merged 4 commits into from
May 24, 2024
Merged

Conversation

yuchenshi
Copy link
Member

@yuchenshi yuchenshi commented May 23, 2024

Description

This PR makes the CLI pass --listen=127.0.0.1:9399,[::1]:9399 instead of --http_port and --grpc_port to the Data Connect emulator.

This removes the spearate GRPC port because a single port can now handle both HTTP and GRPC.

It also fixes a bug where the Data Connect emulator listens on all addresses instead of 127.0.0.1 regardless of config in firebase.json.

Scenarios Tested

Ran a local build of a Data Connect emulator with the CLI and verified it responds to both GRPC and HTTP under both 127.0.0.1:9399 and [::1]:9399.

Sample Commands

firebase emulators:start

@yuchenshi yuchenshi marked this pull request as draft May 23, 2024 01:24
dconeybe added a commit to firebase/firebase-android-sdk that referenced this pull request May 23, 2024
In cl/636395279 the grpc and http ports were combined. Also, that CL changes the --grpc_port and --http_port flags into a single flag: --listen, whose default value is 127.0.0.1:3628. This change will be included in the v1.1.18 release of the Data Connect emulator, and will be used by firestore-tools with firebase/firebase-tools#7211 merged in.
@yuchenshi yuchenshi marked this pull request as ready for review May 24, 2024 00:55
@yuchenshi yuchenshi enabled auto-merge (squash) May 24, 2024 00:55
@yuchenshi yuchenshi merged commit 5463aad into master May 24, 2024
35 checks passed
@yuchenshi yuchenshi deleted the ys/dataconnect-listen branch May 24, 2024 01:07
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