-
Notifications
You must be signed in to change notification settings - Fork 248
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(rtdb): add go rtdb emulator support #517
feat(rtdb): add go rtdb emulator support #517
Conversation
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. |
if strings.Contains(rawEmulatorHostURL, ".") { | ||
namespace = strings.Split(rawEmulatorHostURL, ".")[0] | ||
} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I observed similar behaviour in the Node sdk: https://github.com/firebase/firebase-admin-node/blob/master/src/database/database.ts#L325
func parseEmulatorHost(rawEmulatorHostURL string, parsedEmulatorHost *url.URL) (*dbURLConfig, error) { | ||
if strings.Contains(rawEmulatorHostURL, "//") { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I saw this behavior in the Python sdk: https://github.com/firebase/firebase-admin-python/blob/37ecf18d0ad09d7402143f22443e957a57ec7a2a/firebase_admin/db.py#L784
I've signed the CLA check and I'm not sure why other PRs appear to have this same issue: |
Thank you for kicking off CI |
@r-LaForge I have seen this happen if we switch the base branch from to |
@lahirumaramba Rebase shows branch is up to date
Branching directly from |
Thank you so much @r-LaForge for your contribution! We will include this change in the next release of the Admin Go SDK. |
Discussion
This PR hopes to add support for the RTDB Emulator, and is discussed here: #298
There was a previous attempt here: #471
The Python and Node SDK emulation was followed as an example.
Testing
Unit tests have been added to ensure the emulator config (url + namespace) is set correctly. All others still pass.
I have also adjusted the integration tests to be able to run against the emulator.
Simply run:
Please note the
FIREBASE_DATABASE_EMULATOR_NAMESPACE
is only used to set the database name during tests.API Changes
No public API changes.
RELEASE NOTE: The Realtime Database API now supports running against Firebase emulator suite. Enable the emulator mode by setting the
FIREBASE_DATABASE_EMULATOR_HOST
environment variable to point to the emulator Database endpoint.Fixes: #298