Updating the test suite to use firebase-admin #93
Conversation
1 similar comment
There was a problem hiding this comment.
Had a few docs suggestions, but the code changes looked good - and fairly minimal, which is great! The only other thing I think may be worth mentioning in the docs is that if developers are running the queue on GCP infrastructure, users can make use of admin.credential.applicationDefault() and won't even need to bring their own service account.
| @@ -44,6 +44,8 @@ queue | |||
|
|
|||
| See [Custom references to tasks and specs](#custom-references-to-tasks-and-specs) for defining the locations of these other than the default. | |||
|
|
|||
| Firebase Queue works with a Firebase database reference from either the [firebase-admin](https://www.npmjs.com/package/firebase-admin) (for admin access) or [firebase](https://www.npmjs.com/package/firebase) (for end-user access) NPM module, though it is mainly intended to perform administrative actions. Check out [this blog post](https://firebase.googleblog.com/2016/11/bringing-firebase-to-your-server.html) for an introduction to firebase-admin. | |||
There was a problem hiding this comment.
s/database/Database/(that's the official way we write it in our docs)- Put backticks around
firebase-admin(two instances) andfirebase(one instance) s/NPM/npm(official name is lowercase)
| firebase.initializeApp({ | ||
| serviceAccount: 'path/to/serviceAccountCredentials.json', | ||
| admin.initializeApp({ | ||
| credential: admin.credential.cert('path/to/serviceAccountCredentials.json'), |
There was a problem hiding this comment.
People have been tripped up by the fact that the link here is relative to where the script is run. I've updated the official docs to make this more foolproof:
var serviceAccount = require('path/to/serviceAccountCredentials.json');
admin.initializeApp({
credential: admin.credential.cert('path/to/serviceAccountCredentials.json'),
databaseURL: '<your-database-url>'
});
| firebase.initializeApp({ | ||
| serviceAccount: 'path/to/serviceAccountCredentials.json', | ||
| admin.initializeApp({ | ||
| credential: admin.credential.cert('path/to/serviceAccountCredentials.json'), |
| firebase.initializeApp({ | ||
| serviceAccount: 'path/to/serviceAccountCredentials.json', | ||
| admin.initializeApp({ | ||
| credential: admin.credential.cert('path/to/serviceAccountCredentials.json'), |
|
Application default credentials are mentioned in the blog post linked. These docs are mainly for the initial development of a queue where you're unlikely to have such credentials installed. |
|
|
||
| ```js | ||
| // my_queue_worker.js | ||
|
|
||
| var Queue = require('firebase-queue'); | ||
| var admin = require('firebase-admin'); | ||
|
|
||
| var serviceAccount = require('path/to/serviceAccountCredentials.json') |
There was a problem hiding this comment.
Nit: missing semicolon (same with the two other places you have this).
Fixes #90
NPM (at least v3.10.8) seems to install optional dependencies by default which is not what we want I don't think. I think it's clear from the documentation that you always have to install firebase-queue alongside either firebase-admin or firebase, so let's just remove it from anything other than the devDependencies.