-
Notifications
You must be signed in to change notification settings - Fork 771
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
Pass throught custom http headers as transport configuration #1630
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks good to me overall; inline comment about an additional check.
autobahn/wamp/component.py
Outdated
@@ -160,6 +160,8 @@ def _create_transport(index, transport, check_native_endpoint=None): | |||
raise ValueError( | |||
'options must be a dict, not {}'.format(type(options)) | |||
) | |||
|
|||
headers = transport.get("headers", None) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Probably want a check in "rawsocket" (i.e. headers
should be None
if kind == 'rawsocket'
because headers doesn't make sense for that case).
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
thanks a lot! I like the "minimalistic" approach. +1 for the additional check that @meejah mentioned as this is a transport-specific feature while "component" is exactly about abstracting away "all" transport details (like in general with WAMP at the app level, above transports). for me, a simple/deadly "assert" would be fine .. that's enough to make it fail and the user/developer aware.
rgd pls see TLDR: this is fallout from a breaking change in pytest-asyncio ... |
I will do the polishing next week. |
Here we go, I raised a ValueError, as for the other incompatible arguments and removed the default None return value from get(). |
flake is complaining about WS https://github.com/crossbario/autobahn-python/actions/runs/8357659564/job/22881485753?pr=1630#step:6:71 the asyncio issue is related to the latest (breaking) changes in pytest-asyncio rgd event loop setup I guess .. |
autobahn/wamp/component.py
Outdated
@@ -229,6 +229,8 @@ def _create_transport(index, transport, check_native_endpoint=None): | |||
endpoint_config = transport['endpoint'] | |||
if 'serializers' in transport: | |||
raise ValueError("'serializers' is only for websocket; use 'serializer'") | |||
if headers is None: |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
is not None
I think..?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
oh man...
thanks
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
should be fixed
|
rebased to master and rerunning CI .. let's see |
merged via #1638 - thanks for contributing! |
This minimalistic PR adds the possibility to specify custom http headers as transport configuration.
Example:
The patch does not change the internal logic, the configuration is automatically passed and used in the websocket connection factory. I tried to keep the changes minimal (although I was tempted to apply the boy scout rule and do some tidy up work).