remove: typeName from CfnAlertConfigurationProps

swachter commented 2 months ago

Proposed changes

Remove the CfnAlertConfigurationProps.typeName property. It is not mentioned in the createAlertConfiguration rest API and not forwarded by the corresponding CloudFormation resource.

Unfortunately, this would be a breaking change. Maybe the property should be deprecated first and removed later.

Type of change:

[ ] Bug fix (non-breaking change which fixes an issue)
[ ] New feature (non-breaking change which adds functionality)
[x] Breaking change (fix or feature that would cause existing functionality to not work as expected)
[ ] This change requires a documentation update

Required Checklist:

[x] I have signed the MongoDB CLA
[ ] I have added tests that prove my fix is effective or that my feature works
[ ] I have added any necessary documentation (if appropriate)
[ ] I have run make fmt and formatted my code
[ ] I have tested the CDK constructor in a CFN stack. See TESTING.md
[ ] If changes include removal or addition of 3rd party GitHub actions, I updated our internal document. Reach out to the APIx Integration slack channel to get access to the internal document.

github-actions[bot] commented 1 month ago

This PR has gone 30 days without any activity and meets the project’s definition of "stale". This will be auto-closed if there is no new activity over the next 30 days. If the issue is still relevant and active, you can simply comment with a "bump" to keep it open, or add the label "not_stale". Thanks for keeping our repository healthy!

StefanWachter1507 commented 1 month ago

bump

oarbusi commented 1 month ago

Thanks @swachter for opening this PR and contributing. The changes in CDK must be done running cdk.sh script with the cloudformation submodule updated. There are in fact some attributes that are removed(FlowName and FlowdockApiToken) doing this, but not typeName. This means that typeName is used by the CFN resource. In the API you linked I see that typeName is mentioned and is inside the notifications attribute. Please let me know if there is any further concern and I will help if possible

StefanWachter1507 commented 1 month ago

Then, the top-level typeName property must be removed from the CFN resource. It is not used in the resource implementation.

Shall I open a PR in the mongodbatlas-cloudformation-resources repo? I assume that the typeName block at mongodb-atlas-alertconfiguration.json#L71 must be removed.

oarbusi commented 1 month ago

Then, the top-level typeName property must be removed from the CFN resource. It is not used in the resource implementation.

Shall I open a PR in the mongodbatlas-cloudformation-resources repo? I assume that the typeName block at mongodb-atlas-alertconfiguration.json#L71 must be removed.

@StefanWachter1507 yes you are right, this should be removed from the CFN resource. We can do this ourselves and I will let you know once the PR is ready and also once we release the new version with the fix

maastha commented 3 weeks ago

Created CLOUDP-252262 to look into removing unused attributes in this resource. Closing this issue for now, we'll update once the change is released.

Thanks again for creating the PR!

mongodb / awscdk-resources-mongodbatlas