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

feat(data_classes): Add CloudWatchAlarmEvent data class #3868

Conversation

par6n
Copy link
Contributor

@par6n par6n commented Feb 28, 2024

Issue number: #3848

Summary

AWS has recently introduced AWS Lambda as an alarm state change action (link to the news entry). This pull request facilitates reading the event that a CloudWatch Alarm sends to a Lambda function.

Changes

Please provide a summary of what's being changed

  • Introduces a new data class: CloudWatchAlarmEvent, along with its satellite data classes and enums.

Pending items:

  • Test the class against complex alarms, for example, the ones with mathematical expressions.

User experience

Please share what the user experience looks like before and after this change

You will be able to deserialize the CloudWatch Alarm state change event. Docstrings are added for fields.

from aws_lambda_powertools.utilities.data_classes import event_source, CloudWatchAlarmEvent

@event_source(data_class=CloudWatchAlarmEvent)
def lambda_handler(event: CloudWatchAlarmEvent, context):
    if event.state.value.name == 'ALARM':
        print(f"{event.alarm_arn} is on alarm...")

Checklist

If your change doesn't seem to apply, please leave them unchecked.

Is this a breaking change?

RFC issue number:

Checklist:

  • Migration process documented
  • Implement warnings (if it can live side by side)

Acknowledgment

By submitting this pull request, I confirm that you can use, modify, copy, and redistribute this contribution, under the terms of your choice.

Disclaimer: We value your time and bandwidth. As such, any pull requests created on non-triaged issues might not be successful.

@boring-cyborg boring-cyborg bot added the tests label Feb 28, 2024
Copy link

boring-cyborg bot commented Feb 28, 2024

Thanks a lot for your first contribution! Please check out our contributing guidelines and don't hesitate to ask whatever you need.
In the meantime, check out the #python channel on our Powertools for AWS Lambda Discord: Invite link

@pull-request-size pull-request-size bot added the size/L Denotes a PR that changes 100-499 lines, ignoring generated files. label Feb 28, 2024
@rubenfonseca rubenfonseca marked this pull request as ready for review February 29, 2024 09:56
@rubenfonseca rubenfonseca requested a review from a team as a code owner February 29, 2024 09:56
@github-actions github-actions bot added the feature New feature or functionality label Feb 29, 2024
@boring-cyborg boring-cyborg bot added the documentation Improvements or additions to documentation label Feb 29, 2024
@par6n
Copy link
Contributor Author

par6n commented Feb 29, 2024

@rubenfonseca I'm done with the changes. I've tried running make lint-docs, however it exits with code zero, without returning any errors. I suppose that means no linting issues were found?

@github-actions github-actions bot removed the documentation Improvements or additions to documentation label Feb 29, 2024
@leandrodamascena
Copy link
Contributor

Looking at this

Copy link
Contributor

@leandrodamascena leandrodamascena left a comment

Choose a reason for hiding this comment

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

Hello @par6n! Thank you for sending this OR! This is great work.

I made some comments to improve the customer experience and hope we can address them soon and merge this PR!

docs/utilities/data_classes.md Outdated Show resolved Hide resolved
tests/events/cloudWatchAlarmEvent.json Outdated Show resolved Hide resolved
@boring-cyborg boring-cyborg bot added the documentation Improvements or additions to documentation label Feb 29, 2024
@par6n
Copy link
Contributor Author

par6n commented Feb 29, 2024

@leandrodamascena Thanks for the excellent comments! I'm not a Python-savvy, and I've already learned a few things working on this.

@leandrodamascena
Copy link
Contributor

@leandrodamascena Thanks for the excellent comments! I'm not a Python-savvy, and I've already learned a few things working on this.

Ohhh really? No one can say that you haven't written code in Python for a long time, the code is very clean and well documented.

I'm checking the documentation and I see some fields that are missing, for example when you have a composite alarm you have additional fields, for example:

'previousState': {
      'value': 'ALARM',
      'reason': 'arn:aws:cloudwatch:us-east-1:111122223333:alarm:CompositeDemo.FirstChild transitioned to ALARM at Friday 04 August, 2023 12:54:46 UTC',
      'reasonData': '{"triggeringAlarms":[{"arn":"arn:aws:cloudwatch:us-east-1:111122223333:alarm:CompositeDemo.FirstChild","state":{"value":"ALARM","timestamp":"2023-08-04T12:54:46.138+0000"}}]}',
      'timestamp': '2023-08-04T12:54:46.138+0000',
      'actionsSuppressedBy': 'WaitPeriod',
      'actionsSuppressedReason': 'Actions suppressed by WaitPeriod'
    },

It's very late here and I'm wrapping up the day. Please give me until tomorrow to push some commits that address certain issues. I will explain the details of the fixes that I address.

Please let me know if you have any other questions in the meantime.

Thanks again.

@pull-request-size pull-request-size bot added size/XL Denotes a PR that changes 500-999 lines, ignoring generated files. and removed size/L Denotes a PR that changes 100-499 lines, ignoring generated files. labels Mar 11, 2024
Copy link

sonarcloud bot commented Mar 11, 2024

Quality Gate Passed Quality Gate passed

Issues
0 New issues
0 Accepted issues

Measures
0 Security Hotspots
No data about Coverage
2.9% Duplication on New Code

See analysis details on SonarCloud

@leandrodamascena
Copy link
Contributor

Hello @par6n! I pushed a commit with a fix for a few small things and here is the list of what I changed:

1 - I removed the CloudWatchAlarmStateValue and CloudWatchAlarmActionSuppressor variables. We are only using this once and can add the type annotation directly to the property.
2 - I optimized some "ifs" to make the code cleaner.
3 - Our intention is to allow the client to access the properties following the same structure they have in JSON, therefore, I changed the location of some fields and created two new classes: CloudWatchAlarmMetricStat and CloudWatchAlarmConfiguration.
4 - I modified the test a little to make it more realistic.

Thank you for your outstanding work on this task. Before merging, I'm sending this PR to @rubenfonseca for review. If you have any questions or need clarification, please feel free to reach out to me.

@github-actions github-actions bot removed the documentation Improvements or additions to documentation label Mar 11, 2024
@par6n
Copy link
Contributor Author

par6n commented Mar 11, 2024

@leandrodamascena I checked out your changes, and it all looks good to me. Excellent work over there! If there's anything I could do further for this PR, please let me know!

@rubenfonseca
Copy link
Contributor

Reviewing now

Copy link
Contributor

@leandrodamascena leandrodamascena left a comment

Choose a reason for hiding this comment

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

APPROVED!! @par6n

@leandrodamascena leandrodamascena merged commit d2ab095 into aws-powertools:develop Mar 12, 2024
17 checks passed
Copy link

boring-cyborg bot commented Mar 12, 2024

Awesome work, congrats on your first merged pull request and thank you for helping improve everyone's experience!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
feature New feature or functionality size/XL Denotes a PR that changes 500-999 lines, ignoring generated files. tests
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants