powertools-lambda-python icon indicating copy to clipboard operation
powertools-lambda-python copied to clipboard

feat(nested-event-sources): Generic unwrapping of event source data

Open seshubaws opened this issue 1 year ago • 6 comments

Issue number: https://github.com/aws-powertools/powertools-lambda-python/issues/2678

Summary

Changes

Please provide a summary of what's being changed

User experience

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

Checklist

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

Is this a breaking change?

RFC issue number: https://github.com/aws-powertools/powertools-lambda-python/issues/2678

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.

seshubaws avatar Apr 03 '24 22:04 seshubaws

@rafaelgsr @leandrodamascena Please take a look at this PR and let me know if I need to change anything. I will work on fixing the linting soon. I had a couple general questions as well:

  1. Should we take every instance of a class implementing the DictWrapper class and change it to the EventWrapper class (since EventWrapper now implements DictWrapper and EventWrapper has the parent class implementation of how to unwrap nested events).
  2. Since events using the Firehose Event class have the option to encode the data, should we add a flag to auto decode the data on unwrapping? Otherwise, it would return encoded data and that data would not be able to unwrap to the next level like you see in the firehose_sns_event in nested_test_events.py (https://github.com/aws-powertools/powertools-lambda-python/pull/4069/files#diff-c01fab973bea227eb17986da25f24715386579f71e68297cd520929431bc0778R5-R14).

seshubaws avatar Apr 26 '24 19:04 seshubaws

Discussed the above questions:

  1. Should we take every instance of a class implementing the DictWrapper class and change it to the EventWrapper class (since EventWrapper now implements DictWrapper and EventWrapper has the parent class implementation of how to unwrap nested events).

Probably ok to do this, but @leandrodamascena will confirm.

  1. Since events using the Firehose Event class have the option to encode the data, should we add a flag to auto decode the data on unwrapping? Otherwise, it would return encoded data and that data would not be able to unwrap to the next level like you see in the firehose_sns_event in nested_test_events.py (https://github.com/aws-powertools/powertools-lambda-python/pull/4069/files#diff-c01fab973bea227eb17986da25f24715386579f71e68297cd520929431bc0778R5-R14).

Decided we should just decode without needing a flag since customers will always need to decode their data. Still discussing how to decode for different data classes.

seshubaws avatar May 02 '24 16:05 seshubaws

@leandrodamascena think this is something @sthulb could review and take it to the finish line?

heitorlessa avatar Jun 05 '24 11:06 heitorlessa

@leandrodamascena think this is something @sthulb could review and take it to the finish line?

Hello @heitorlessa! I completely missed that comment. @seshubaws has done a great job so far and we can help finalize this PR and launch this new feature.

I'll assing to Simon after we finish some internal things related to V3.

leandrodamascena avatar Jul 22 '24 23:07 leandrodamascena