Skip to content
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

Add proxy variable #9739

Open
wants to merge 3 commits into
base: master
Choose a base branch
from
Open

Add proxy variable #9739

wants to merge 3 commits into from

Conversation

jhqwqmc
Copy link

@jhqwqmc jhqwqmc commented Feb 22, 2024

Summary

Add the use of the proxy variable to the from_url function of the webhook class.

During use, I found that the from_url function did not provide a proxy parameter to pass in. My area requires a proxy to access discord, so I added this parameter.

Checklist

  • If code changes were made then they have been tested.
    • I have updated the documentation to reflect the changes.
  • This PR fixes an issue.
  • This PR adds something new (e.g. new method or parameters).
  • This PR is a breaking change (e.g. methods or parameters removed/renamed)
  • This PR is not a code change (e.g. documentation, README, ...)

Add the use of the proxy variable to the from_url function of the webhook class.

During use, I found that the from_url function did not provide a proxy parameter to pass in. My area requires a proxy to access discord, so I added this parameter.
@No767
Copy link

No767 commented Feb 22, 2024

If you were to ask me, this isn't needed. I would wonder why even support proxies for webhooks?

@Rapptz
Copy link
Owner

Rapptz commented Feb 22, 2024

The reason for this is because you can set the proxies on the ClientSession you create instead.

@Rapptz Rapptz closed this Feb 22, 2024
@jhqwqmc
Copy link
Author

jhqwqmc commented Feb 22, 2024

ClientSession does not provide proxy variables. You need to pass proxy variables to Webhook.

@jhqwqmc
Copy link
Author

jhqwqmc commented Feb 22, 2024

Needing a proxy is also a helpless move. Our country has blocked the website discord.com.

@Rapptz
Copy link
Owner

Rapptz commented Feb 22, 2024

Actually it seems you're right, I forgot this change happened to aiohttp a while back. This PR is insufficient though. It needs to support proxy_auth parameter as well and be properly documented.

@Rapptz Rapptz reopened this Feb 22, 2024
Added `proxy_auth` variable
@jhqwqmc
Copy link
Author

jhqwqmc commented Feb 22, 2024

I just added proxy_auth

@Rapptz
Copy link
Owner

Rapptz commented Feb 22, 2024

This is still missing documentation, and most likely needs to run black.

Add description
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.

None yet

3 participants