You signed in with another tab or window. Reload to refresh your session.You signed out in another tab or window. Reload to refresh your session.You switched accounts on another tab or window. Reload to refresh your session.Dismiss alert
{{ message }}
Repository navigation
Static typing: resolved_headers_field is not Optional #4147
Expression of type "Dict[str, Any] | None" cannot be assigned to declared type "dict[str, list[str]]"
Type "Dict[str, Any] | None" cannot be assigned to type "dict[str, list[str]]"
"None" is incompatible with "dict[str, list[str]]"
Hi @wurstnase I'm not sure I follow this one. resolved_headers_field is currently marked as returning an Optional, meaning that in some cases it can return None. Shouldn't you handle that in your code instead of changing the API signature?
One could argue that checking the implementation, the field is always present. But due to Hyrum's Law I would prefer to add this to our v3 branch (more on that earlier next week).
Everyone - appreciate the help and cautious in raising the bar for everyone's experience. Merged, ran e2e tests, and pushed additional fixes to complete the PR.
Bottom line: headers are present, when it is not we default to {}, so we were overly cautious on Optional there.
Activity
rubenfonseca commentedon Apr 18, 2024
Hi @wurstnase I'm not sure I follow this one.
resolved_headers_fieldis currently marked as returning an Optional, meaning that in some cases it can return None. Shouldn't you handle that in your code instead of changing the API signature?One could argue that checking the implementation, the field is always present. But due to Hyrum's Law I would prefer to add this to our v3 branch (more on that earlier next week).
@leandrodamascena I would appreciate your thoughts here too
leandrodamascena commentedon Apr 18, 2024
Yes! I agree with you @rubenfonseca that we should avoid doing this now and tag this issue/pr as eligible for Powertools V3.
rubenfonseca commentedon Apr 18, 2024
Hope that's ok with you @wurstnase. I'll add this to our v3 queue. I promise v3 is not far ahead ;)
tonnico commentedon Apr 18, 2024
@rubenfonseca @leandrodamascena
I can't see how this code ever can become
None?As you can see, finally we return a dict at any time. It is impossible to return None.
If something is
Noneit will raise an exception.At least this could become an empty dict, but never None.
heitorlessa commentedon May 7, 2024
Nico is correct,
headerswill be an empty dict in the worst case -powertools-lambda-python/aws_lambda_powertools/utilities/data_classes/common.py
Line 101 in 4e32f08
That said, there's a few other places for its associated PR to be statically correct., Sending fixes today
heitorlessa commentedon May 7, 2024
fixes pushed, tests suite ran successfully. Pending PR CI suite
github-actions commentedon May 7, 2024
This issue is now closed. Please be mindful that future comments are hard for our team to see.
If you need more assistance, please either tag a team member or open a new issue that references this one.
If you wish to keep having a conversation with other community members under this issue feel free to do so.
heitorlessa commentedon May 7, 2024
Everyone - appreciate the help and cautious in raising the bar for everyone's experience. Merged, ran e2e tests, and pushed additional fixes to complete the PR.
Bottom line:
headersare present, when it is not we default to{}, so we were overly cautious onOptionalthere.