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

Rotating proxy functionality #139

Merged

Conversation

andreademasi
Copy link
Contributor

@andreademasi andreademasi commented Jan 10, 2022

This PR addresses issues regarding rate limiting by using a socks5 rotating proxy derived from a list of proxies fetched from proxyscan.io.
May fix #137 and #115
Main issue is proxy quality, some of them are slow or not working.

TODO:

  • Proxy checker
  • Add option for configuring through config.yml

@andreademasi andreademasi changed the title Rotating proxt functionality Rotating proxy functionality Jan 10, 2022
@andreademasi andreademasi force-pushed the feature/rotating-proxy branch 3 times, most recently from 43c5403 to 78a5cfd Compare January 11, 2022 08:11
src/gateio_new_coins_announcements_bot/main.py Outdated Show resolved Hide resolved
src/gateio_new_coins_announcements_bot/main.py Outdated Show resolved Hide resolved
src/rotating_proxy.py Outdated Show resolved Hide resolved
src/rotating_proxy.py Outdated Show resolved Hide resolved
src/rotating_proxy.py Outdated Show resolved Hide resolved
src/rotating_proxy.py Outdated Show resolved Hide resolved
@Linus045
Copy link
Collaborator

Linus045 commented Jan 15, 2022

You somehow changed the line-endings think, which marked the whole file as changed, can you fix that, so only your actual changes are marked.

@andreademasi
Copy link
Contributor Author

andreademasi commented Jan 15, 2022

@Linus045 Fixed

@andreademasi andreademasi marked this pull request as ready for review January 15, 2022 22:10
logger.error(e)

# Merging old proxies with new ones
_list = list(proxy_res[:-1].split("\n") | _proxy_list.keys())
Copy link
Collaborator

Choose a reason for hiding this comment

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

If the try..except above triggers proxy_res will be an invalid value and therefore cause another exception here when trying proxy_res[:-1] wouldn't it?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I'll add a return statement in the except clause

src/gateio_new_coins_announcements_bot/rotating_proxy.py Outdated Show resolved Hide resolved
src/gateio_new_coins_announcements_bot/rotating_proxy.py Outdated Show resolved Hide resolved
src/gateio_new_coins_announcements_bot/rotating_proxy.py Outdated Show resolved Hide resolved
src/gateio_new_coins_announcements_bot/rotating_proxy.py Outdated Show resolved Hide resolved
src/gateio_new_coins_announcements_bot/rotating_proxy.py Outdated Show resolved Hide resolved
src/gateio_new_coins_announcements_bot/rotating_proxy.py Outdated Show resolved Hide resolved
@andreademasi andreademasi force-pushed the feature/rotating-proxy branch 2 times, most recently from 20c5f7c to 8a98ba8 Compare February 3, 2022 22:53
src/gateio_new_coins_announcements_bot/rotating_proxy.py Outdated Show resolved Hide resolved


def init_proxy():
threading.Thread(target=lambda: _every(60 * 10, _fetch_proxies)).start()
Copy link
Collaborator

Choose a reason for hiding this comment

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

we might wanna make the poll delay also configurable via the config.
But for now its fine, after some more testing we can think about it again.

proxy = get_proxy()
logger.debug(f"Using proxy: {proxy}")
try:
response = self.http_client.get(request_url, proxies={"http": "socks5://" + proxy})
Copy link
Collaborator

Choose a reason for hiding this comment

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

Another point we should consider is what happens when the request via the proxy fails or returns status code 429.
In that case we probably wanna remove the proxy or don't use it for some time (e.g. 5min).
But I don't mind implementing that in another PR considering that this is an opt-in feature via the config anyway.

threads: list[threading.Thread] = []
try:
proxy_res = requests.get(
"https://www.proxyscan.io/api/proxy?last_check=180&limit=20&type=socks5&format=txt&ping=1000"
Copy link
Collaborator

Choose a reason for hiding this comment

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

we might also wanna consider a different last_check value, but thats something we can adjust after more testing as well.
(For reference: https://www.proxyscan.io/api)

src/gateio_new_coins_announcements_bot/rotating_proxy.py Outdated Show resolved Hide resolved

if rotating_proxy_is_ready():
proxy = get_proxy()
logger.debug(f"Using proxy: {proxy}")
Copy link
Collaborator

Choose a reason for hiding this comment

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

I think this should be a logger.info combined with the first message e.g.:
when using proxy: "Getting Binance announcements [Using proxy: 127.0.0.1]"
when no valid proxy exists: "Getting Binance announcements [No proxy available]"
when the proxy feature is disabled: "Getting Binance announcements"

That being said, it should be implemented in a new PR and not here.
I was planning on reworking the console logs at some point anyway so we should consider this then as well.

src/gateio_new_coins_announcements_bot/rotating_proxy.py Outdated Show resolved Hide resolved
@andreademasi
Copy link
Contributor Author

I'm thinking it might be useful to filter proxies to countries close to japan (where Binance's servers are located) like China, Thailand, Singapore, South Korea, Hong Kong, and of course Japan.
This can be easily achieved in the API call: https://www.proxyscan.io/api/proxy?last_check=180&country=cn,th,sg,kr,hk,jp&limit=20&type=socks5&format=txt&ping=1000

@Linus045 what are your thoughts on this?

@Linus045
Copy link
Collaborator

Linus045 commented Feb 8, 2022

I'm thinking it might be useful to filter proxies to countries close to japan (where Binance's servers are located) like China, Thailand, Singapore, South Korea, Hong Kong, and of course Japan. This can be easily achieved in the API call: https://www.proxyscan.io/api/proxy?last_check=180&country=cn,th,sg,kr,hk,jp&limit=20&type=socks5&format=txt&ping=1000

@Linus045 what are your thoughts on this?

I agree, but I think this should be done in a new PR.
It should be an option in the options.yml similar to the other stuff I mentioned above.

@Linus045
Copy link
Collaborator

Linus045 commented Feb 8, 2022

This looks good to me.
Small problems and the other stuff discussed in the comments will be fixed/integrated in new PRs.

@Linus045 Linus045 merged commit 16629d0 into CyberPunkMetalHead:develop Feb 8, 2022
@andreademasi andreademasi deleted the feature/rotating-proxy branch February 8, 2022 17:24
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