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

Bug: DictWrapper should fully implement Mapping interface #1503

Closed
Tankanow opened this issue Sep 6, 2022 · 3 comments
Closed

Bug: DictWrapper should fully implement Mapping interface #1503

Tankanow opened this issue Sep 6, 2022 · 3 comments
Assignees
Labels
bug Something isn't working

Comments

@Tankanow
Copy link
Contributor

Tankanow commented Sep 6, 2022

Expected Behaviour

DictWrapper and all of its subclasses should fully implement the Mapping interface so that it can be used interchangeably with code that expects the event to be a dictionary. This helps Powertools Middleware play nicely with existing middleware.

Current Behaviour

DictWrapper does not implement the abstract __iter__ and __len__ methods from the Mapping interface. Currently, client code cannot iterate over any subclass of DictWrapper, nor can it use the dictionary built-ins keys, values, or items.

Code snippet

from aws_lambda_powertools.utilities.data_classes.common import DictWrapper

event = DictWrapper({'foo': 'bar'})

[k for k in event]  # raises KeyError: 0
# all of the following raise AttributeError: 'DictWrapper' object has no attribute '...'
event.keys()
event.items()
event.values()

Possible Solution

DictWrapper should subclass either Mapping or Dict, for example:

class DictWrapper(Mapping):
    """Provides a single read only access to a wrapper dict"""

    def __init__(self, data: Dict[str, Any]):
        self._data = data

    def __getitem__(self, key: str) -> Any:
        return self._data[key]

    def __eq__(self, other: Any) -> bool:
        if not isinstance(other, DictWrapper):
            return False

        return self._data == other._data

    def get(self, key: str) -> Optional[Any]:
        return self._data.get(key)

    @property
    def raw_event(self) -> Dict[str, Any]:
        """The original raw event dict"""
        return self._data

    # missing methods
    def __iter__(self) -> Iterator:
        return iter(self._data)

    def __len__(self) -> int:
        return len(self._data)

Steps to Reproduce

see comment above

AWS Lambda Powertools for Python version

latest

AWS Lambda function runtime

3.9

Packaging format used

PyPi

Debugging logs

No logs here. I'm happy to submit the PR to fix this. This should be a non-breaking change.
@Tankanow Tankanow added bug Something isn't working triage Pending triage from maintainers labels Sep 6, 2022
@rubenfonseca rubenfonseca self-assigned this Sep 12, 2022
@rubenfonseca rubenfonseca added area/event_sources and removed triage Pending triage from maintainers labels Sep 12, 2022
@rubenfonseca
Copy link
Contributor

Hi @Tankanow thank you for the very clear explanation! Would you mind opening a PR with the changes, together with a couple of tests? We will be thrilled to review it and include it in our next release.

@Tankanow
Copy link
Contributor Author

@rubenfonseca, happy to do so. I'll get to it this week. 👍🏽

@github-actions
Copy link
Contributor

github-actions bot commented Oct 5, 2022

This is now released under 1.30.0 version!

@github-actions github-actions bot closed this as completed Oct 5, 2022
@github-actions github-actions bot removed the pending-release Fix or implementation already in dev waiting to be released label Oct 5, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
Development

No branches or pull requests

3 participants