-
Notifications
You must be signed in to change notification settings - Fork 1.7k
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
Add resource name_from_id
provider-defined function
#10106
Add resource name_from_id
provider-defined function
#10106
Conversation
name_from_id
provider-defined function
I can't see checks for running tests etc... /gcbrun |
This comment was marked as outdated.
This comment was marked as outdated.
This comment was marked as outdated.
This comment was marked as outdated.
This comment was marked as outdated.
This comment was marked as outdated.
e2ca443
to
bffe4cb
Compare
Rebased to include the latest commits from main that were synced into the feature branch. This should address VCR failures above. |
This comment was marked as outdated.
This comment was marked as outdated.
This comment was marked as outdated.
This comment was marked as outdated.
This comment was marked as outdated.
This comment was marked as outdated.
bffe4cb
to
3a4e3ee
Compare
Tests analyticsTotal tests: Click here to see the affected service packagesall service packages are affected Action takenFound 1 affected test(s) by replaying old test recordings. Starting RECORDING based on the most recent commit. Click here to see the affected testsTestAccProviderFunction_name_from_id |
|
3a4e3ee
to
354e3f4
Compare
Tests analyticsTotal tests: Click here to see the affected service packagesall service packages are affected
|
This comment was marked as outdated.
This comment was marked as outdated.
This comment was marked as outdated.
This comment was marked as outdated.
This comment was marked as outdated.
This comment was marked as outdated.
Tests analyticsTotal tests: Click here to see the affected service packagesall service packages are affected Action takenFound 1 affected test(s) by replaying old test recordings. Starting RECORDING based on the most recent commit. Click here to see the affected testsTestAccKMSEkmConnection_kmsEkmConnectionBasicExample_update |
|
Hi there, I'm the Modular magician. I've detected the following information about your changes: Diff reportYour PR generated some diffs in downstreams - here they are. Terraform GA: Diff ( 5 files changed, 323 insertions(+), 2 deletions(-)) |
Tests analyticsTotal tests: Click here to see the affected service packagesall service packages are affected Action takenFound 2 affected test(s) by replaying old test recordings. Starting RECORDING based on the most recent commit. Click here to see the affected testsTestAccDataSourceGoogleServiceAccountIdToken_impersonation|TestAccNotebooksInstance_notebookInstanceFullExample |
|
Redistributing review as I've used a lot of Shuya's time recently. @c2thorn could you please review this PR? 🙏 It's very similar to the region/zone one you previously reviewed, but now we're pulling out the short name of a resource from the long id/URL. The regex for this is more permissive though, so there might be edge cases to test for/add extra validation for. I've triggered tests to run for this branch at : https://hashicorp.teamcity.com/buildConfiguration/TerraformProviders_GoogleCloud_FEATUREBRANCHPROVIDERFUNCTIONS_MM_GA_GOOGLE_PACKAGE_FUNCTIONS/118301?hideTestsFromDependencies=false&hideProblemsFromDependencies=false&expandBuildChangesSection=true |
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.
Looks good overall, just some minor questions
f6c50c4
into
FEATURE-BRANCH-provider-functions
This is the final PR for adding provider defined functions that enable pulling these values out of identifiers:
I've taken into account feedback from other PRs for this PR, e.g. updating docs and acc test resources used.
This function is special as it pulls out the end of an input string starting at the last
/
. There isn't an easy consistent regex to use, versus/projects/{project}
Release Note Template for Downstream PRs (will be copied)