-
Notifications
You must be signed in to change notification settings - Fork 1.6k
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
Allow setting SqlServerUserDetails from google_sql_user resource #6016
Conversation
Thanks for your pull request! It looks like this may be your first contribution to a Google open source project. Before we can look at your pull request, you'll need to sign a Contributor License Agreement (CLA). For more information, open the CLA check for this pull request. |
Hello! I am a robot who works on Magic Modules PRs. I've detected that you're a community contributor. @rileykarson, a repository maintainer, has been assigned to assist you and help review your changes. ❓ First time contributing? Click here for more detailsYour assigned reviewer will help review your code by:
You can help make sure that review is quick by running local tests and ensuring they're passing in between each push you make to your PR's branch. Also, try to leave a comment with each push you make, as pushes generally don't generate emails. If your reviewer doesn't get back to you within a week after your most recent change, please feel free to leave a comment on the issue asking them to take a look! In the absence of a dedicated review dashboard most maintainers manage their pending reviews through email, and those will sometimes get lost in their inbox. |
The provider crashed while running the VCR tests in REPLAYING mode |
The provider crashed while running the VCR tests in REPLAYING mode |
Co-authored-by: Riley Karson <[email protected]>
The provider crashed while running the VCR tests in REPLAYING mode |
I believe this is ready to go, although I cannot see why the VCR-test is failing, and if that is related to my change or not. Let me know if anything else is needed. |
Sorry for the delay in getting back! Here's the failure:
|
Hi! I'm the modular magician. Your PR generated some diffs in downstreams - here they are. Diff report:Terraform GA: Diff ( 5 files changed, 74 insertions(+), 8 deletions(-)) |
Tests analyticsTotal tests: Action takenTriggering VCR tests in RECORDING mode for the following tests that failed during VCR: TestAccCloudAssetFolderFeed_cloudAssetFolderFeedExample |
Resolved that issue, i believe - another test failed, but i cannot see the output. Please advise |
That one's just noise! |
Hi! I'm the modular magician. Your PR generated some diffs in downstreams - here they are. Diff report:Terraform GA: Diff ( 4 files changed, 116 insertions(+), 12 deletions(-)) |
Tests analyticsTotal tests: Action takenTriggering VCR tests in RECORDING mode for the following tests that failed during VCR: TestAccCloudAssetFolderFeed_cloudAssetFolderFeedExample |
Co-authored-by: Riley Karson <[email protected]>
Co-authored-by: Riley Karson <[email protected]>
Hi! I'm the modular magician. Your PR generated some diffs in downstreams - here they are. Diff report:Terraform GA: Diff ( 4 files changed, 112 insertions(+), 10 deletions(-)) |
Tests analyticsTotal tests: Action takenTriggering VCR tests in RECORDING mode for the following tests that failed during VCR: TestAccFirebaserulesRelease_BasicRelease|TestAccCloudAssetFolderFeed_cloudAssetFolderFeedExample|TestAccSqlUser_mysqlDisabled |
Tests passed during RECORDING mode: Tests failed during RECORDING mode: Please fix these to complete your PR |
Hi! I'm the modular magician. Your PR generated some diffs in downstreams - here they are. Diff report:Terraform GA: Diff ( 2 files changed, 103 insertions(+), 4 deletions(-)) |
Tests analyticsTotal tests: Action takenTriggering VCR tests in RECORDING mode for the tests that failed during VCR. Click here to see the failed testsTestAccEventarcTrigger_BasicHandWritten|TestAccIAM2DenyPolicy_iamDenyPolicyBasicExample|TestAccSqlUser_mysqlDisabled |
I believe the only remaining failure is unrelated to this change, is it possible to get this merged, so we can use workload identity via terraform? |
LGTM, yep! Sorry for the delay in getting back, been pretty underwater in reviews. |
@rileykarson - I noticed on the terraform provider docs page that it doesn't mention the Is that documentation managed here, and is that manually maintained? Just curious if another PR to update the documentation would be in order? |
Yeah, we'd need to manually change that page. I missed that in the initial review! For most resources it's automated these days. |
Hey just chiming in here that sql_server_user_details would be super useful if it wasn't read only and actually documented. i'm imported a database dump from |
Allow setting SqlServerUserDetails from google_sql_user resource
This is a lift of hashicorp/terraform-provider-google#11342 into the magic modules repo.
Resolves hashicorp/terraform-provider-google#10438
If this PR is for Terraform, I acknowledge that I have:
make test
andmake lint
to ensure it passes unit and linter tests.Release Note Template for Downstream PRs (will be copied)