Skip to content

add environment variable to point admin sdk at rtdb emulator #589

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

Closed
wants to merge 10 commits into from

Conversation

jmwski
Copy link
Contributor

@jmwski jmwski commented Jul 3, 2019

Hey there! So you want to contribute to a Firebase SDK?
Before you file this pull request, please read these guidelines:

Discussion

  • Read the contribution guidelines (CONTRIBUTING.md).
  • If this has been discussed in an issue, make sure to link to the issue here.
    If not, go file an issue about this before creating a pull request to discuss.

Testing

  • Make sure all existing tests in the repository pass after your change.
  • If you fixed a bug or added a feature, add a new test to cover your code.

API Changes

  • At this time we cannot accept changes that affect the public API. If you'd like to help
    us make Firebase APIs better, please propose your change in an issue so that we
    can discuss it together.

@jmwski jmwski requested a review from samtstern July 3, 2019 21:55
@jmwski jmwski self-assigned this Jul 3, 2019
@jmwski jmwski requested review from hiranya911 and samtstern July 18, 2019 17:59
@jmwski
Copy link
Contributor Author

jmwski commented Jul 19, 2019

Abandoned and replaced by #596

@jmwski jmwski closed this Jul 19, 2019
@jmwski jmwski deleted the wyszynski/rtdb-emulator-host-envvar branch July 19, 2019 17:20
@ryanpbrewster ryanpbrewster restored the wyszynski/rtdb-emulator-host-envvar branch July 29, 2019 18:22
@ryanpbrewster
Copy link
Contributor

I think we need to re-open this PR and discuss using fake credentials for the emulators. Right now we're requiring developers to either do this themselves, or use a real production service account credential.

* set `databaseURL` in FirebaseAppOptions. The varaible should be a complete
* URI specifying a transfer protocol, hostname, and port number:
*
* FIREBASE_DATABASE_EMULATOR_HOST=http://localhost:9000
Copy link
Contributor

Choose a reason for hiding this comment

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

If I am not mistaken, the protocol is omitted in FIRESTORE_EMULATOR_HOST. I see that it might make sense to include it here (since we need to include the namespace). Should we come up with a consistent pattern that apploes to both RTDB and Firestore? @ryanpbrewster

Copy link
Contributor

@ryanpbrewster ryanpbrewster Jul 30, 2019

Choose a reason for hiding this comment

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

the protocol should not be included in the FIRESTORE_EMULATOR_HOST variable.

Copy link
Contributor

Choose a reason for hiding this comment

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

Do we care about the consistency between FIRESTORE_EMULATOR_HOST and FIREBASE_DATABASE_EMULATOR_HOST?

Copy link
Contributor

Choose a reason for hiding this comment

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

Yes, ideally neither should include the protocol.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

This comment is dated, neither emulator host variable has a protocol.

@dungahk
Copy link

dungahk commented Nov 10, 2019

I totally agree with reopening this, or at least reverting #602

I am wondering how a new feature in the Firebase JS SDK is sufficient for the firebase-admin Node SDK?

For example, I am using firebase-admin with firebase-functions and I do not touch the firebase JS client SDK and having this in the admin SDK would be really appreciated.

The only weird thing I found is that if I run the emulators using emulators:start and manually invoke the functions it seems to work, but if I try to automate the steps using emulators:exec it doesn't work.

@samtstern
Copy link
Contributor

@dungahk can you open a new issue on the firebase-tools repo describing the issue you're having with emulators:exec versus emulators:start?

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.

6 participants