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

feat: [UIE-8006] - DBaaS 2.0 Create #10872

Merged

Conversation

corya-akamai
Copy link
Contributor

Description πŸ“

DBaaS 2.0 enhancements to create page

Changes πŸ”„

  • New 2 node option for dedicated CPUs
  • Expose handleTabChange in PanelsPanel to show/hide 2 node option

Target release date πŸ—“οΈ

9/10/24

How to test πŸ§ͺ

Prerequisites

  • "Managed Databases V2" capability
  • dbaasV2 flag enabled

Verification steps

  • 2 node option will appear on create page

As an Author I have considered πŸ€”

Check all that apply

  • πŸ‘€ Doing a self review
  • ❔ Our contribution guidelines
  • 🀏 Splitting feature into small PRs
  • βž• Adding a changeset
  • πŸ§ͺ Providing/Improving test coverage
  • πŸ” Removing all sensitive information from the code and PR description
  • 🚩 Using a feature flag to protect the release
  • πŸ‘£ Providing comprehensive reproduction steps
  • πŸ“‘ Providing or updating our documentation
  • πŸ•› Scheduling a pair reviewing session
  • πŸ“± Providing mobile support
  • β™Ώ Providing accessibility support

@corya-akamai corya-akamai requested a review from a team as a code owner September 3, 2024 15:07
@corya-akamai corya-akamai requested review from mjac0bs and abailly-akamai and removed request for a team September 3, 2024 15:07
@mjac0bs mjac0bs added the DBaaS Relates to Database as a Service label Sep 3, 2024
@abailly-akamai
Copy link
Contributor

@corya-akamai thx for the contribution! How does one get the "Managed Databases V2" capability?

Copy link

github-actions bot commented Sep 3, 2024

Coverage Report: βœ…
Base Coverage: 86.21%
Current Coverage: 86.27%

@corya-akamai
Copy link
Contributor Author

corya-akamai commented Sep 3, 2024

@corya-akamai thx for the contribution! How does one get the "Managed Databases V2" capability?
@abailly-akamai,
dchavan can give you access, or adding it to the Legacy mock data accountFactory capabilities

Copy link
Contributor

@abailly-akamai abailly-akamai left a comment

Choose a reason for hiding this comment

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

Looks good and works as described.

create-database e2e failing is suspicious approving confirming this test.

Let a couple other comments for consideration

Copy link
Contributor

@mjac0bs mjac0bs left a comment

Choose a reason for hiding this comment

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

Hey @corya-akamai - Alban is right that the Cypress failure for create-database.spec.ts does seem related here. That test fails locally for me consistently for all 3 test cases at the point where it's trying to await the mocked createDatabase request. I don't know what the issue is here right off - I confirmed can still create a database in the UI by clicking the button. And although we're not checking for 2 Nodes as a option, 1 Node and 3 Nodes are still present and selectable in the test. (Maybe @AzureLatte or @jdamore-linode can spot what I'm overlooking.)

Screenshot 2024-09-03 at 2 21 20β€―PM

Other than that, this PR looked good - I had some minor comments on test clean up, thank you for the coverage.

@corya-akamai
Copy link
Contributor Author

corya-akamai commented Sep 4, 2024

Hey @corya-akamai - Alban is right that the Cypress failure for create-database.spec.ts does seem related here. That test fails locally for me consistently for all 3 test cases at the point where it's trying to await the mocked createDatabase request. I don't know what the issue is here right off - I confirmed can still create a database in the UI by clicking the button. And although we're not checking for 2 Nodes as a option, 1 Node and 3 Nodes are still present and selectable in the test. (Maybe @AzureLatte or @jdamore-linode can spot what I'm overlooking.)

Screenshot 2024-09-03 at 2 21 20β€―PM

Other than that, this PR looked good - I had some minor comments on test clean up, thank you for the coverage.

@mjac0bs,

The error was due to shared mock data https://github.com/linode/manager/pull/10872/files#r1744096806. The create-database.spec is now passing. I noticed the resize-database.spec is failing, but that seems to be failing back to v1.21.0 when it was added https://github.com/linode/manager/pull/10461/files. Maybe @sujai-git or stayal can fix. I tried

beforeEach(() => {
    mockAppendFeatureFlags({
      databaseResize: makeFeatureFlagData(true),
    });
    mockGetFeatureFlagClientstream();
});

but that didn't seem to resolve the issue.

@mjac0bs mjac0bs self-requested a review September 4, 2024 18:32
Copy link
Contributor

@mjac0bs mjac0bs left a comment

Choose a reason for hiding this comment

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

@corya-akamai Good find and fix on the Capability - that makes sense.

I went back and looked at the CI e2e run and noticed it's resize-linode.spec.ts, not to be confused with resize-database.spec.ts, that is failing (as well as longview.spec.ts). I wouldn't expect the changes in this PR to have any impact on those tests, nor have I seen them fail consistently in CI recently, so I think those failures were just flukes.

Just for confirmation's sake, all of the following passed for me locally. Unit tests are fine as well; the code coverage action was just skipped.

Screenshot 2024-09-05 at 4 30 49β€―PM
Screenshot 2024-09-05 at 4 38 33β€―PM
Screenshot 2024-09-05 at 4 42 33β€―PM
Screenshot 2024-09-05 at 4 32 29β€―PM

(I'm going to go ahead and merge this now.)

@mjac0bs mjac0bs added the Approved Multiple approvals and ready to merge! label Sep 5, 2024
@mjac0bs mjac0bs merged commit ad03a23 into linode:develop Sep 5, 2024
17 of 19 checks passed
@corya-akamai corya-akamai deleted the UIE-8006-dbaas-enhancements-create branch September 6, 2024 16:59
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Approved Multiple approvals and ready to merge! DBaaS Relates to Database as a Service
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants