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

better handle failed requests when listing functions for backends #5111

Merged
merged 2 commits into from Oct 11, 2022

Conversation

bkendall
Copy link
Contributor

Description

The check that was added to catch permissions issues had a little problem and the status wasn't being properly passed through and checked. This CL fixes both and adds additional tests.

Scenarios Tested

Deploying with the cloud functions API turned off when including rewrite functions works correctly.

Fixes #5071.

Hopefully for real.

@codecov-commenter
Copy link

codecov-commenter commented Oct 11, 2022

Codecov Report

Base: 56.01% // Head: 56.09% // Increases project coverage by +0.07% 🎉

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

Additional details and impacted files
@@            Coverage Diff             @@
##           master    #5111      +/-   ##
==========================================
+ Coverage   56.01%   56.09%   +0.07%     
==========================================
  Files         308      308              
  Lines       20699    20700       +1     
  Branches     4187     4188       +1     
==========================================
+ Hits        11594    11611      +17     
+ Misses       8102     8082      -20     
- Partials     1003     1007       +4     
Impacted Files Coverage Δ
src/deploy/hosting/convertConfig.ts 69.90% <0.00%> (+0.97%) ⬆️
src/gcp/cloudfunctions.ts 69.18% <0.00%> (+2.88%) ⬆️
src/emulator/auth/state.ts 85.43% <0.00%> (+0.56%) ⬆️
src/gcp/cloudfunctionsv2.ts 62.10% <0.00%> (+4.21%) ⬆️

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
Member

@inlined inlined left a comment

Choose a reason for hiding this comment

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

I might have modified the v2 codebase to also log & wrap if necessary, but I understand making minimal changes after freeze.

@bkendall bkendall merged commit b50317a into master Oct 11, 2022
@bkendall bkendall deleted the bk-5071 branch October 11, 2022 23:38
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Failed to deploy to hosting with 'Failed to list functions for project' error
4 participants