-
Notifications
You must be signed in to change notification settings - Fork 4.1k
Fix wrong database name on existing database error for New-AzSqlDatabaseSecondary #12421
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
Conversation
|
Can one of the admins verify this patch? |
|
Hi @bradrich-msft , Is there any issue for this PR? |
No. |
|
Hi @wyunchi-ms |
| // The database already exists | ||
| throw new PSArgumentException( | ||
| string.Format(Resources.DatabaseNameExists, this.DatabaseName, this.PartnerServerName), | ||
| string.Format(Resources.DatabaseNameExists, GetEffectivePartnerDatabaseName(this.DatabaseName, this.PartnerDatabaseName), this.PartnerServerName), |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks for fixing this!
This will tell the user that "Database already exists" for the secondary (partner) database without mentioning that is failing because of the partner database. I wonder how much work it is to add a new error message (something like "PartnerDatabaseNameExists" instead of using the existing DatabaseNameExists
| .SYNOPSIS | ||
| Tests creating a named secondary database of already existing database | ||
| #> | ||
| function Test-CreateNamedSecondaryDatabaseNegative() |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Is this the only negative test? In case we add more negative tests, can we be more specific like TestNegative-CreateNamedSecondaryAlreadyExists or something similar to indicate the test scenario?
I was wondering whether there is a issue for this. It's OK now. |
Description
New-AzSqlDatabaseSecondary cmdlet was returning the wrong database name in existing database error in some cases.
This fixes that by using the correct method to get database name. This was missed in earlier change.
Checklist
CONTRIBUTING.mdChangeLog.mdfile(s) has been updated:ChangeLog.mdfile can be found atsrc/{{SERVICE}}/{{SERVICE}}/ChangeLog.md## Upcoming Releaseheader -- no new version header should be added