Skip to content

Conversation

@xeviknal-cf
Copy link

Hello,

Shouldn't HTTP Headers be a dict[str, str] instead of [str, List[str]]?

Orkes UI also complains about this:
image

And I receive this headers:
image

Either the issue is in the typing or marshalling the data.

Co-authored-by: Xavier Canal Masjuan <xeviknal@gmail.com>
@xeviknal-cf
Copy link
Author

cc @IgorChvyrov-sm @nthmost-orkes

method: HttpMethod = HttpMethod.GET,
uri: Optional[str] = None,
headers: Optional[Dict[str, List[str]]] = None,
headers: Optional[Dict[str, str]] = None,
Copy link
Collaborator

Choose a reason for hiding this comment

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

This could break backward compatibility for clients that already use this. I would recommend setting it up as

headers: Optional[dict[str, str|list[str]]]

and then on line 59
self._headers = ",".join(headers) if isinstance(headers, list) else headers

perhaps?

This will not work for Set-Cookie headers as the separators are semi-colons, something that likely should be handled with another parameter.

method: HttpMethod = HttpMethod.GET,
uri: Optional[str] = None,
headers: Optional[Dict[str, List[str]]] = None,
headers: Optional[Dict[str, str]] = None,
Copy link
Collaborator

Choose a reason for hiding this comment

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

Same comment as above.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants