-
Notifications
You must be signed in to change notification settings - Fork 180
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: Correctly handle GCP updates of mongodbatlas_network_peering
#2306
Conversation
APIx bot: a message has been sent to Docs Slack channel |
@@ -142,6 +142,52 @@ func TestAccNetworkRSNetworkPeering_basicGCP(t *testing.T) { | |||
}) | |||
} | |||
|
|||
func TestAccNetworkRSNetworkPeering_updateBasicGCP(t *testing.T) { | |||
acc.SkipTestForCI(t) // needs GCP configuration |
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 it doable to enable in CI or it's very complicated?
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.
I have run the tests but I have done it using my GCP account. If we are able to have a generic account for the team we could do it, but I didn't want to use my account credentials to run the tests in CI.
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.
that's for sure. maybe we can create a triage ticket to have test credentials in GCP
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.
I will create the ticket! Thanks
edit: created CLOUDP-250829
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.
LGTM
@@ -142,6 +142,52 @@ func TestAccNetworkRSNetworkPeering_basicGCP(t *testing.T) { | |||
}) | |||
} | |||
|
|||
func TestAccNetworkRSNetworkPeering_updateBasicGCP(t *testing.T) { |
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.
just to confirm, with the previous implementation this test was failing in the patch request right?
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.
Yes exactly. It was failing because update is not possible if the Status is Available
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.
LGTM 👍
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.
LGTM / 1 nit
Co-authored-by: kanchana-mongodb <[email protected]>
Description
Correctly handle GCP updates of
mongodbatlas_network_peering
Link to any related issue(s): CLOUDP-250605
Type of change:
Required Checklist:
Further comments