fix(event_handlers): omit explicit None HTTP header values #1793
Conversation
leandrodamascena
requested review from
heitorlessa
and removed request for
a team
pull-request-size
bot
added
the
size/M
| @@ -65,8 +66,9 @@ def serialize(self, headers: Dict[str, Union[str, List[str]]], cookies: List[Coo | |||
| if isinstance(values, str): | |||
| payload[key].append(values) | |||
| else: | |||
| for value in values: | |||
| payload[key].append(value) | |||
| if values: | |||
There was a problem hiding this comment.
when you accept a falsy value but not null, check for null otherwise a X-Is-Admin: false will be omitted here. For example, if values is None, would work in both cases.
| payload[key].append(value) | ||
| if values: | ||
| for value in values: | ||
| payload[key].append(value) |
There was a problem hiding this comment.
we can drop an additional for loop by using list.extend, as it accepts an Iterable and it'll add all elements at the end of the list. This drops the complexity
|
Last missing piece is docs. We should mention somewhere that custom headers with |
Codecov ReportBase: 97.58% // Head: 97.58% // Increases project coverage by
Additional details and impacted files@@ Coverage Diff @@
## develop #1793 +/- ##
========================================
Coverage 97.58% 97.58%
========================================
Files 141 141
Lines 6420 6425 +5
Branches 440 442 +2
========================================
+ Hits 6265 6270 +5
Misses 123 123
Partials 32 32
Help us with your feedback. Take ten seconds to tell us how you rate us. Have a feature suggestion? Share it here. ☔ View full report at Codecov. |
heitorlessa
changed the title
heitorlessa
changed the title
Issue number: #1791
Summary
Changes
Add a fix for when header is None and avoid iterator or len issues
User experience
Although not recommended by the RFC that defines headers in the HTTP protocol, the user can force a header with a value of None. To avoid this problem the header which has value None will be omitted from the response
Checklist
If your change doesn't seem to apply, please leave them unchecked.
Is this a breaking change?
RFC issue number:
Checklist:
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.