Skip to content
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

⭐ Create AWS resource aws.ec2.instances {iamInstanceProfile} #4767

Merged
merged 20 commits into from
Oct 25, 2024

Conversation

mm-weber
Copy link
Contributor

@mm-weber mm-weber commented Oct 18, 2024

This adds the following resource:
instanceProfileResource

Signed-off-by: Manuel Weber <[email protected]>
Signed-off-by: Manuel Weber <[email protected]>
Signed-off-by: Manuel Weber <[email protected]>
Copy link
Contributor

github-actions bot commented Oct 18, 2024

Test Results

3 116 tests  ±0   3 115 ✅ ±0   1m 21s ⏱️ -1s
  371 suites ±0       1 💤 ±0 
   28 files   ±0       0 ❌ ±0 

Results for commit 0fb614d. ± Comparison against base commit 88ea761.

♻️ This comment has been updated with latest results.

Signed-off-by: Manuel Weber <[email protected]>
Signed-off-by: Manuel Weber <[email protected]>
Signed-off-by: Manuel Weber <[email protected]>
Copy link
Contributor

@vjeffrey vjeffrey left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

this works well!

i would prefer if the instance profile was a real resource instead of a dict, i left a longer comment explaining my thinking, but putting my +1 here now in case we need this quickly - it does the job, i just think if we make it a real resource we'll avoid a little bit of rework later

@tas50
Copy link
Member

tas50 commented Oct 23, 2024

+1 to having real resources vs. dicts. It makes for better auto completes and docs. Overall it's a better user experience even if it's more work for us.

Copy link
Contributor

@misterpantz misterpantz left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Some docs suggestions for you, @mm-weber!

providers/aws/resources/aws.lr Outdated Show resolved Hide resolved
providers/aws/resources/aws.lr Outdated Show resolved Hide resolved
providers/aws/resources/aws.lr Outdated Show resolved Hide resolved
providers/aws/resources/aws.lr Outdated Show resolved Hide resolved
providers/aws/resources/aws.lr Outdated Show resolved Hide resolved
providers/aws/resources/aws.lr Outdated Show resolved Hide resolved
Signed-off-by: Manuel Weber <[email protected]>

This comment has been minimized.

This comment has been minimized.

@mm-weber
Copy link
Contributor Author

mm-weber commented Oct 25, 2024

Thank you all for your help on this @tas50 @misterpantz and especially @vjeffrey for walking me through her process !

Done some cleanup and fixed a little thing, think it's ready to go now.

image

@mm-weber mm-weber merged commit 82339d9 into main Oct 25, 2024
15 checks passed
@mm-weber mm-weber deleted the manuel/aws-IamInstanceProfile branch October 25, 2024 22:11
@github-actions github-actions bot locked and limited conversation to collaborators Oct 25, 2024
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants