-
Notifications
You must be signed in to change notification settings - Fork 39
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
Able to assign host
and port
for emualtor
#35
base: main
Are you sure you want to change the base?
Conversation
|
Hey thanks for this contribution. The admin SDKs don't set the host/port directly like the client SDKs, instead they read from a environment variable, see https://firebase.google.com/docs/emulator-suite/connect_auth#admin_sdks We should probably instead parse this value if it exists and pass it through. |
Hey, thanks for the doc. I didn't know that because I am using glcoud to start firestore emulatpr which doesn't require the firebase env. I notice you has the "do_not_use_environment" lint rule set. In order to follow this, I just enhanced the [Emulator] class that it can be initiated from envString like this.
for my gcloud use case, I do this,
What do you think about this? |
That's not the same "enrivonment". We're talking about using Also, this would need testing :) |
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.
Cf above
…on of tests for Emulator related things.
Hi @rrousselGit , thanks for the feedback. |
return const Emulator._defaultAuth(); | ||
} else { | ||
return Emulator.fromEnvString( | ||
Platform.environment['FIREBASE_AUTH_EMULATOR_HOST']!, |
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.
Could you remove this !
?
This is possible by doing something like:
final env = Platform.environment['FIREBASE_AUTH_EMULATOR_HOST'];
if (env != null) {...}
else {...}
return const Emulator._defaultFirestore(); | ||
} else { | ||
return Emulator.fromEnvString( | ||
Platform.environment['FIRESTORE_EMULATOR_HOST']!, |
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.
Same here
void useAuthEmulator({ | ||
Emulator? emulator | ||
}) { |
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.
Wouldn't a simpler API be this?
void useAuthEmulator([Uri? uri])
We can then remove that Emulator
class.
Emulator? emulator | ||
}) { | ||
_isUsingAuthEmulator = true; | ||
emulator ??= Emulator.auth(); |
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'd rather have the environment logic directly in here, instead of a constructor inside a separate Emulator class.
Beside setting the
host
andport
. TheuseEmulator
function is still functioning the same.