-
Notifications
You must be signed in to change notification settings - Fork 132
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
CodeGuru Profiler Profiling Group #1316
Conversation
Hey @chlunde, Thank you for opening this PR, I took a quick look at your PR:
|
@turkenf thanks for looking at the draft, I just pushed it to sync between two computers and didn't have time to test it yet :) But with IdentifierFromProvider and the following example: apiVersion: codeguruprofiler.aws.upbound.io/v1beta1
kind: ProfilingGroup
metadata:
annotations:
meta.upbound.io/example-id: codeguruprofiler/v1beta1/profilinggroup
labels:
testing.upbound.io/example-name: example
name: example
spec:
forProvider:
agentOrchestrationConfig:
- profilingEnabled: true
computePlatform: Default
name: example2
region: us-west-2 I get
If I use the code as the PR is now, it works fine:
|
@turkenf should we do something about the array here?
|
@turkenf I rebased the PR due to conflict. If I try diff --git a/config/externalname.go b/config/externalname.go
index b51b69f2e..16b93fdb5 100644
--- a/config/externalname.go
+++ b/config/externalname.go
@@ -70,7 +70,7 @@ var TerraformPluginFrameworkExternalNameConfigs = map[string]config.ExternalName
// CodeGuru Profiler
// Profiling Group can be imported using the profiling group name
- "aws_codeguruprofiler_profiling_group": config.NameAsIdentifier,
+ "aws_codeguruprofiler_profiling_group": config.IdentifierFromProvider,
}
// TerraformPluginSDKExternalNameConfigs contains all external name configurations And the following managed resource (with added spec.forProvider.name), it fails: apiVersion: codeguruprofiler.aws.upbound.io/v1beta1
kind: ProfilingGroup
metadata:
annotations:
meta.upbound.io/example-id: codeguruprofiler/v1beta1/profilinggroup
labels:
testing.upbound.io/example-name: example
name: example
spec:
forProvider:
name: example3
agentOrchestrationConfig:
- profilingEnabled: true
computePlatform: Default
region: us-west-2 The code as it is in this PR seems to work fine, as you can see in the screenshot. Am I missing something here?
|
/test-examples="examples/codeguruprofiler/v1beta1/profilinggroup.yaml" |
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.
Hi @chlunde,
Many thanks for your patience and effort here. I left a comment for you to consider.
Hi @chlunde, The |
@turkenf Thanks! Sorry for the late reply, I've been traveling. I had to rebase due to conflicts. I also added |
/test-examples="examples/codeguruprofiler/v1beta1/profilinggroup.yaml" |
Actually, you are right @chlunde, I misunderstood the situation here 🤦 I suggested the After this enlightenment, I think it is more appropriate to use Sorry for the misunderstanding, if you don't mind, could you please set the external name configuration to |
Signed-off-by: Carl Henrik Lunde <chlunde@ifi.uio.no>
Signed-off-by: Carl Henrik Lunde <chlunde@ifi.uio.no>
@turkenf cool! I've updated (and retested) |
/test-examples="examples/codeguruprofiler/v1beta1/profilinggroup.yaml" |
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.
Many thanks @chlunde 🙌
Description of your changes
Fixes #959
I have:
make reviewable
to ensure this PR is ready for review.backport release-x.y
labels to auto-backport this PR if necessary.How has this code been tested
A successful uptest run: https://github.com/crossplane-contrib/provider-upjet-aws/actions/runs/9398769793