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

Remove skipped resources workaround #109

Merged
merged 4 commits into from
Jan 28, 2025

Conversation

sk593
Copy link
Contributor

@sk593 sk593 commented Jan 13, 2025

This reverts the change implemented in #96 since the schema has been updated.

Signed-off-by: sk593 <[email protected]>
@sk593 sk593 requested review from a team as code owners January 13, 2025 22:35
"AWS::SageMaker::Cluster": {},
}
// These will be reverted when the issue is resolved, but the map will be kept for future use if needed.
var skippedResources = map[string]struct{}{}
Copy link
Contributor Author

@sk593 sk593 Jan 13, 2025

Choose a reason for hiding this comment

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

The resources were removed but I kept the map so we don't need to reimplement the logic if we run into the same issue in the future. I'm open to removing it but I kept it as a "just in case" since it's an error that AWS would need to fix, not us.

willdavsmith
willdavsmith previously approved these changes Jan 27, 2025
@kachawla
Copy link
Contributor

aws/aws-sdk-go-v2#2913 is still open. Did they fix it without closing the issue?

@sk593
Copy link
Contributor Author

sk593 commented Jan 27, 2025

aws/aws-sdk-go-v2#2913 is still open. Did they fix it without closing the issue?

The issue was from the CFN API, not the AWS SDK so they wouldn't be able to fix it. I did submit another issue here in case that raised awareness: aws-cloudformation/cloudformation-coverage-roadmap#2213

It seems like the CFN API was updated in between and removed the missing schema errors but I'm not sure whether that info was passed down to the other repos.

Signed-off-by: sk593 <[email protected]>
@sk593 sk593 dismissed stale reviews from willdavsmith and brooke-hamilton via 965fd59 January 27, 2025 20:00
@sk593 sk593 merged commit ef7d35d into radius-project:main Jan 28, 2025
4 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants