Skip to content

Conversation

@fredzqm
Copy link
Contributor

@fredzqm fredzqm commented Jun 5, 2024

Ran through the onboard flow today.

The existing output made me feel compelled to sit there and wait for firebase init to finish.
Though customers can totally start coding right away.

  • Make firebase init dataconnect not waiting for Cloud SQL instance.
  • Make CLI catch any state=failed cloud sql state. Currently, we show those as succeed, but it's actually completely busted and unusable.
  • Use global listServices, greatly reduce number of requests to the backend.

Scenarios Tested

firebase init takes over a "fresh from console" service.
It spits out a link to monitor Cloud SQL provision instead of a forever long spinner that drives developer away.

Screenshot 2024-06-04 at 11 14 39 PM

Then, rerun firebase init while the Cloud SQL instance is being created.

Screenshot 2024-06-04 at 11 14 10 PM

Then attempt firebase deploy while the Cloud SQL instance is being created.
Screenshot 2024-06-04 at 8 24 10 PM

firebase init when Cloud SQL instance is in a failed state (ran into this while deleting and recreating the instance)
Screenshot 2024-06-04 at 11 09 00 PM

@fredzqm fredzqm requested a review from joehan June 5, 2024 03:10
@fredzqm fredzqm assigned joehan and unassigned joehan Jun 5, 2024
@fredzqm fredzqm removed the request for review from joehan June 5, 2024 03:13
@fredzqm fredzqm changed the title Hint to customers that canceling command while Cloud SQL is creating is fine Improve init flow edge case when Cloud SQL is PENING_CREATE Jun 5, 2024
@fredzqm fredzqm changed the title Improve init flow edge case when Cloud SQL is PENING_CREATE Make firebase init dataconnect not wait for Cloud SQL instances provision Jun 5, 2024
@codecov-commenter
Copy link

Codecov Report

Attention: Patch coverage is 15.38462% with 11 lines in your changes missing coverage. Please review.

Project coverage is 54.03%. Comparing base (06cb833) to head (67feebb).
Report is 25 commits behind head on master.

Files Patch % Lines
src/dataconnect/provisionCloudSql.ts 0.00% 6 Missing ⚠️
src/gcp/cloudsql/cloudsqladmin.ts 28.57% 5 Missing ⚠️
Additional details and impacted files
@@            Coverage Diff             @@
##           master    #7275      +/-   ##
==========================================
- Coverage   54.06%   54.03%   -0.04%     
==========================================
  Files         386      386              
  Lines       26137    26156      +19     
  Branches     5370     5378       +8     
==========================================
+ Hits        14131    14133       +2     
- Misses      10742    10759      +17     
  Partials     1264     1264              

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

Copy link
Contributor

@joehan joehan left a comment

Choose a reason for hiding this comment

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

LGTM after changes

@joehan joehan enabled auto-merge (squash) June 6, 2024 20:02
@joehan joehan merged commit 2559f4d into master Jun 6, 2024
@fredzqm fredzqm deleted the fz/comment branch June 6, 2024 20:14
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.

3 participants