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

Locally serving Hosting proxies Cloud Run rewrites #1187

Merged
merged 14 commits into from
Apr 11, 2019
Merged

Conversation

Memeriaj
Copy link
Contributor

@Memeriaj Memeriaj commented Apr 9, 2019

When running firebase serve the local Hosting instance will proxy Cloud Run rewrites configured in the firebase.json file to the Cloud Run service that is running in the current project. In the process I refactored out the "proxy" code from locally serving Functions into a shared file.

@Memeriaj Memeriaj self-assigned this Apr 9, 2019
@googlebot googlebot added the cla: yes Manual indication that this has passed CLA. label Apr 9, 2019
@coveralls
Copy link

coveralls commented Apr 9, 2019

Coverage Status

Coverage increased (+1.5%) to 61.71% when pulling 8ddeb1c on am-serve-cr into 25a5d44 on master.

@Memeriaj Memeriaj assigned mbleigh and bkendall and unassigned Memeriaj Apr 9, 2019
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.

It would be fantastic if there was a functional test for this, but I realize that that's more difficult than it may seem. Otherwise, LGTM (assuming things work 🙂)

src/hosting/cloudRunProxy.ts Outdated Show resolved Hide resolved
src/hosting/proxy.ts Outdated Show resolved Hide resolved
src/hosting/proxy.ts Outdated Show resolved Hide resolved
src/hosting/cloudRunProxy.ts Outdated Show resolved Hide resolved
src/hosting/cloudRunProxy.ts Outdated Show resolved Hide resolved
src/hosting/cloudRunProxy.ts Show resolved Hide resolved
src/test/hosting/cloudRunProxy.spec.ts Outdated Show resolved Hide resolved
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.

makes way more sense this way. :) Thanks

@Memeriaj Memeriaj merged commit bdf7a1b into master Apr 11, 2019
@Memeriaj Memeriaj deleted the am-serve-cr branch April 11, 2019 16:29
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
cla: yes Manual indication that this has passed CLA.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants