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

Better handling of connection string #7339

Merged
merged 1 commit into from
Jun 18, 2024
Merged

Conversation

joehan
Copy link
Contributor

@joehan joehan commented Jun 17, 2024

Description

This PR does 2 things:

  • Adding Postgres setup to 'setup:emulators:dataconnect'
  • Error out if no connection string is set when we try to connect to postgres.

Scenarios Tested

Running 'firebase setup:emulators:dataconnect' from outside a project directory, then inside a project directory
Screenshot 2024-06-17 at 4 06 06 PM

Try to start emulator with a connection string configured, error out, run the suggested command, and then successfully run the emulator
Screenshot 2024-06-17 at 4 25 21 PM

@joehan joehan requested review from yuchenshi and fredzqm and removed request for yuchenshi June 17, 2024 23:26
message: `What is the connection string of the local Postgres instance you would like to use with the Data Connect emulator?`,
default: defaultConnectionString,
});
options.rc.setDataconnect(localConnectionString);
Copy link
Contributor

Choose a reason for hiding this comment

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

So we decide to keep firebase setup:emulators:dataconnect instead of firebase init emulators dataconnect.

You probably know a lot more than I do on this. Is this a common pattern?

[non-blocking]

init emulators also setup port and other things. Do you think it make sense to include them or somehow re-use?

Screenshot 2024-06-17 at 4 38 40 PM

Copy link
Contributor Author

@joehan joehan Jun 18, 2024

Choose a reason for hiding this comment

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

We have both commands for all of the (downloadable) emulators - I went with suggesting firebase setup:emulators:dataconnect for this one because it was an easier 1 liner than init emulators, and we do not support init emulators <productName> (and doing so would require an API proposal + a surprising amount of rewiring in init)

I did a bit of digging, and the raison d'etre for setup:emulators:product is for use in CI/CD environment set up scripts (ie Docker files or as a cached Github action step). 'init' doesn't work in these environments because it is interactive only. With that in mind, I've gated the prompt so that it only happens in interactive environments.

I omitted port configuration to match other setup:emulators:<product> commands. I also don't think they are relevant for most users.

@joehan joehan merged commit e3a1a67 into master Jun 18, 2024
41 checks passed
joehan added a commit that referenced this pull request Jun 18, 2024
joehan added a commit that referenced this pull request Jun 18, 2024
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.

None yet

2 participants