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

Hosting SSR region #5504

Merged
merged 15 commits into from
Feb 14, 2023
Merged

Hosting SSR region #5504

merged 15 commits into from
Feb 14, 2023

Conversation

chalosalvador
Copy link
Member

Description

Add the ability to set the region of a web framework's generated SSR function.

  • Add initialization step to choose region to host server-side content
  • Validate region in prepareFrameworks
  • Add custom region in functions.yaml and the cloud function

Scenarios Tested

  1. Initialize a firebase hosting project with a web framework
  • Verify that the "choose a region" step is added to the initialization steps.
  • Verify that the firebase.json file includes the frameworksBackend property with the chosen region.
  • Run firebase deploy with and without the necessity of a backend.
    • When deploying with necessity of a backend, verify that the functions.yaml contains the correct region and the server.js file contains the cloud function with the correct region added to it.
  • Verify that the cloud function is actually created in the correct region.
  1. Manually change the region in firebase.json and verify that the CLI asks to delete or keep the current deployed cloud function with the old region.
  2. Set an invalid region the firebase.json and verify that the CLI fails with the appropriate error message.

Sample Commands

- Add initialization step to choose region
- Validate region in prepareFrameworks
- Add custom region in functions.yaml and the cloud function
schema/firebase-config.json Outdated Show resolved Hide resolved
src/frameworks/index.ts Outdated Show resolved Hide resolved
src/frameworks/index.ts Outdated Show resolved Hide resolved
src/frameworks/index.ts Outdated Show resolved Hide resolved
src/init/features/hosting/index.ts Show resolved Hide resolved
src/init/features/hosting/index.ts Outdated Show resolved Hide resolved
@codecov-commenter
Copy link

codecov-commenter commented Feb 13, 2023

Codecov Report

Base: 56.15% // Head: 56.14% // Decreases project coverage by -0.02% ⚠️

Coverage data is based on head (fb62106) compared to base (50340fc).
Patch coverage: 18.18% of modified lines in pull request are covered.

Additional details and impacted files
@@            Coverage Diff             @@
##           master    #5504      +/-   ##
==========================================
- Coverage   56.15%   56.14%   -0.02%     
==========================================
  Files         317      317              
  Lines       21494    21501       +7     
  Branches     4385     4388       +3     
==========================================
+ Hits        12071    12072       +1     
- Misses       8361     8367       +6     
  Partials     1062     1062              
Impacted Files Coverage Δ
src/deploy/functions/runtimes/node/index.ts 38.83% <ø> (ø)
src/frameworks/index.ts 15.33% <18.18%> (-0.03%) ⬇️

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.

chalosalvador and others added 3 commits February 13, 2023 22:34
- Reuse ALLOWED_SSR_REGIONS
- Add default region to prompt
- Add all the frameworksBackend properties to function args.
…e/firebase-tools into chalosalvador-hosting-ssr-region
src/frameworks/index.ts Outdated Show resolved Hide resolved
src/frameworks/index.ts Outdated Show resolved Hide resolved
Copy link
Member

@jamesdaniels jamesdaniels left a comment

Choose a reason for hiding this comment

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

Made a few small changes, let's :shipit:

@jamesdaniels jamesdaniels enabled auto-merge (squash) February 14, 2023 21:33
@jamesdaniels jamesdaniels merged commit d6826ee into master Feb 14, 2023
@jamesdaniels jamesdaniels deleted the chalosalvador-hosting-ssr-region branch February 14, 2023 23:46
export const NODE_VERSION = parseInt(process.versions.node, 10).toString();
export const DEFAULT_REGION = "us-central1";
export const ALLOWED_SSR_REGIONS = [
Copy link

Choose a reason for hiding this comment

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

Hi, is there a motive behind limiting the regions? For example, we currently use europe-west2 (London) but it seems this configuration would be disallowed?

Choose a reason for hiding this comment

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

That's because cloud Functions V2 are only available in those regions.

Choose a reason for hiding this comment

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

@tamerxkilinc This page https://cloud.google.com/functions/docs/locations?hl=en#tier_2_pricing shows that Cloud Functions v2 are currently supported in many more regions. Does that mean we can already use them or are there other limiting factors?

Choose a reason for hiding this comment

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

Should be possible to use the new regions now

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants