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

fix RDS Aurora enrollment security group tips #47930

Merged
merged 1 commit into from
Oct 26, 2024

Conversation

GavinFrazar
Copy link
Contributor

@GavinFrazar GavinFrazar commented Oct 25, 2024

Changelog: Fixed a bug that prevented selecting security groups during the Aurora database enrollment wizard in the web UI.

Fixes security group tips for RDS aurora mysql/postgres. We weren't populating the security groups for aurora cluster before, so it would throw an error when fetching the security groups to select from.

Copy link

This pull request is automatically being deployed by Amplify Hosting (learn more).

Access this pull request here: https://pr-47930.d3pp5qlev8mo18.amplifyapp.com

Copy link

This pull request is automatically being deployed by Amplify Hosting (learn more).

Access this pull request here: https://pr-47930.d212ksyjt6y4yg.amplifyapp.com

@GavinFrazar GavinFrazar force-pushed the gavinfrazar/fix-rds-aurora-security-group-tips branch from de4f3ac to 0b87111 Compare October 25, 2024 18:38
@GavinFrazar GavinFrazar requested a review from greedy52 October 25, 2024 19:27
Copy link
Contributor

@kimlisa kimlisa left a comment

Choose a reason for hiding this comment

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

i dont think we need to be checking if !securityGroups at all, is there a reason why? 🤔
this component SelectSecurityGroups.tsx does the fetching of security groups on intial render.

if the fetchign failed for some reason, then we should not be rendering things that require the security groups to be processed (thats a bug)

general ui pattern looks like this with api fetching:

if attempt.status === processing -> render loader

if attempt.status === failed -> render error banner

if attempt.status === success -> then render the stuff that uses the data we were expecting, since we expect data to be defined here, we dont need to check for optional?

@GavinFrazar GavinFrazar force-pushed the gavinfrazar/fix-rds-aurora-security-group-tips branch 3 times, most recently from 703dad4 to 3fa70d8 Compare October 25, 2024 22:36
@GavinFrazar GavinFrazar requested a review from kimlisa October 25, 2024 22:38
@GavinFrazar GavinFrazar force-pushed the gavinfrazar/fix-rds-aurora-security-group-tips branch from 3fa70d8 to ab2d69f Compare October 25, 2024 23:27
@GavinFrazar GavinFrazar added this pull request to the merge queue Oct 25, 2024
Merged via the queue into master with commit c1f2896 Oct 26, 2024
45 checks passed
@GavinFrazar GavinFrazar deleted the gavinfrazar/fix-rds-aurora-security-group-tips branch October 26, 2024 00:05
@public-teleport-github-review-bot

@GavinFrazar See the table below for backport results.

Branch Result
branch/v16 Create PR

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

Successfully merging this pull request may close these issues.

3 participants