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

Single project mode #4890

Merged
merged 6 commits into from Oct 4, 2022
Merged

Single project mode #4890

merged 6 commits into from Oct 4, 2022

Conversation

christhompsongoogle
Copy link
Contributor

@christhompsongoogle christhompsongoogle commented Aug 16, 2022

Add a singleProjectMode flag to the firebase config which defaults to true (on init and if missing when read).

Pass the flag to firestore in the form of a warning for now.

Requires the latest version of firestore to be cut including CL/466521585

Tested: Manual testing for configuration and firestore emulator.

@codecov-commenter
Copy link

codecov-commenter commented Aug 17, 2022

Codecov Report

Base: 57.29% // Head: 57.27% // Decreases project coverage by -0.02% ⚠️

Coverage data is based on head (bdd3660) compared to base (ade9f5a).
Patch coverage: 0.00% of modified lines in pull request are covered.

Additional details and impacted files
@@            Coverage Diff             @@
##           master    #4890      +/-   ##
==========================================
- Coverage   57.29%   57.27%   -0.03%     
==========================================
  Files         288      288              
  Lines       19039    19046       +7     
  Branches     3769     3773       +4     
==========================================
  Hits        10908    10908              
- Misses       7237     7244       +7     
  Partials      894      894              
Impacted Files Coverage Δ
src/emulator/controller.ts 13.44% <0.00%> (-0.27%) ⬇️
src/emulator/downloadableEmulators.ts 23.30% <ø> (ø)
src/emulator/firestoreEmulator.ts 23.52% <0.00%> (ø)

Help us with your feedback. Take ten seconds to tell us how you rate us. Have a feature suggestion? Share it here.

☔ View full report at Codecov.
📢 Do you have feedback about the report comment? Let us know in this issue.

Copy link
Contributor

@bkendall bkendall left a comment

Choose a reason for hiding this comment

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

Is the update to npm-shrinkwrap intended?

src/firebaseConfig.ts Outdated Show resolved Hide resolved
@christhompsongoogle
Copy link
Contributor Author

christhompsongoogle commented Aug 17, 2022

Is the update to npm-shrinkwrap intended?

Reverted

Copy link
Contributor

@bkendall bkendall left a comment

Choose a reason for hiding this comment

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

Please:

  1. Add a CHANGELOG entry
  2. Fix the CI tests

@@ -100,6 +100,11 @@ export async function doSetup(setup: any, config: any) {
]);
}

// Set the default behavior to be single project mode.
if (setup.config.emulators.singleProjectMode !== undefined) {
Copy link
Contributor

Choose a reason for hiding this comment

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

If singleProjectMode in firebase.json is set to false, this condition means that singleProjectMode will be enabled. I'm sure that's not the intention.

Do you mean to just check if it's truthy?

Suggested change
if (setup.config.emulators.singleProjectMode !== undefined) {
if (setup.config.emulators.singleProjectMode) {

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Fixed, nice find. After some testing this was backwards. The intention here is to set if it it doesn't exist, create the config, otherwise leave it as the user set it. The default should be true, but the current behavior is false.

@christhompsongoogle christhompsongoogle enabled auto-merge (squash) October 4, 2022 20:35
@christhompsongoogle christhompsongoogle merged commit 7d9ece6 into master Oct 4, 2022
@christhompsongoogle christhompsongoogle deleted the singleProjectMode branch October 4, 2022 20:55
@kovan
Copy link

kovan commented Oct 21, 2022

I am getting megabytes and megabytes of log entries with the message:

[2022-10-21T19:43:17.950Z] Multiple projectIds are not recommended in single project mode. Requested project ID ****, but the emulator is configured for ***. This warning will become an error in the future. To opt-out of single project mode add/set the '"single_project_mode": false' property in the firebase.json emulators config

To the point that it is impacting the performance of the build machine.

Anyway, following the message, I have tried setting the mentioned flag "single_project_mode" to false in firebase.json, but the flag seems to be rejected by the emulator.
Investigating a bit more, it looks like the flag it is spelled as "singleProjectMode" in the JSON schema

I haven't yet tested it with that spelling, I will try to report back when I do.

@bkendall
Copy link
Contributor

@kovan please open a new issue with the relevant information. thanks!

@firebase firebase locked as resolved and limited conversation to collaborators Oct 21, 2022
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants