Skip to content

Fix sw8 loss when use aiohttp. #299

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

Merged
merged 3 commits into from
Apr 14, 2023
Merged

Conversation

Forstwith
Copy link
Contributor

@Forstwith Forstwith commented Apr 13, 2023

Fix sw8 loss when use aiohttp.
In /plugins/sw_aiohttp.py
When headers is not MultiDictProxy or MultiDict , headers = CIMultiDict(headers) make the headers not the same object as kwargs['headers'] . Therefore, the carrier information is not added to the actual sent headers

if headers is None:
    headers = kwargs['headers'] = CIMultiDict()
elif not isinstance(headers, (MultiDictProxy, MultiDict)):
    headers = CIMultiDict(headers)
    kwargs['headers'] = headers     # add this line to add headers back.
for item in carrier:
    headers.add(item.key, item.val)

Fix

  • Add a unit test to verify that the fix works.
  • Explain briefly why the bug exists and how to fix it.

@wu-sheng wu-sheng added this to the 1.1.0 milestone Apr 13, 2023
@wu-sheng wu-sheng added the bug Something isn't working label Apr 13, 2023
@wu-sheng
Copy link
Member

You at least missed to update change log file.
Please read PR template.

@Forstwith Forstwith closed this Apr 13, 2023
@wu-sheng
Copy link
Member

Why close?

@Forstwith
Copy link
Contributor Author

Why close?
I want to add a changelog, and then reopen

@Forstwith Forstwith reopened this Apr 13, 2023
@wu-sheng
Copy link
Member

You don't need to close/reopen, just update on your branch.

@Forstwith
Copy link
Contributor Author

Yes, thanks for letting me know that I don't have to close/reopen. I've added the changelog, and reopened.

@Superskyyy
Copy link
Member

Hello,you should move the changelog to 1.1.0 as 1.0.0 was already released.

@Forstwith
Copy link
Contributor Author

Hello,you should move the changelog to 1.1.0 as 1.0.0 was already released.

I saw that @wu-sheng has updated it for me, many thanks. I see that one of the unit tests is failing, is there anything I can do?

@wu-sheng wu-sheng merged commit 0774003 into apache:master Apr 14, 2023
@Forstwith Forstwith deleted the bugfix-aiohttp branch April 14, 2023 06:08
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

Successfully merging this pull request may close these issues.

3 participants