search

claffin / cloudproxy

Hide your scrapers IP behind the cloud. Provision proxy servers across different cloud providers to improve your scraping success.
https://cloudproxy.io/
MIT License
1.4k stars 80 forks source link

Notification of disabled providers in ui #46

Closed kingforaday closed 3 years ago

kingforaday commented 3 years ago
claffin commented 3 years ago

Thanks for your pull request and apologies for my slow review.

I would just like to clarify your second bullet point:

What bug exactly is this fixing? Currently, if the user does not provide a true or false environment var for each provider enabled setting, then it defaults to false. From what I can see, your code just replicates this via a lambda function?

kingforaday commented 3 years ago

No apologies necessary. Thanks for the cool project. Take for example DIGITALOCEAN_ENABLED, prior to the fix if you had any value specified in that environment variable, then it would have been interpreted as enabled. Meaning, if you had DIGITALOCEAN_ENABLED=False, this would have been enabled. If DIGITALOCEAN_ENABLED=b3uh289, this would have been enabled.

I hope that is helpful. Please don't hesitate to reach out if you have any other questions, always happy to help.

claffin commented 3 years ago

I'm not seeing this same issue having tried setting DIGITALOCEAN_ENABLED = b3uh289. DO stays disabled in this scenario. In manager.py, it checks each provider enabled setting is set as True, see below. I'm not sure how you're seeing this issue.

https://github.com/claffin/cloudproxy/blob/de1b45b9543b6900a904e69cabd45f061c0b102b/cloudproxy/providers/manager.py#L34

claffin commented 3 years ago

I've had to hotfix in https://github.com/claffin/cloudproxy/commit/cb989741f7f1c392830f26fd307b00c87b370a2d which now supersedes a part of what you originally included in your PR as I only just realized it fixed a breaking change introduced previously, just needed to get it out asap.

Thanks for raising the bug and fix.