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

Bug fix and list replaced with deque #115

Open
wants to merge 2 commits into
base: main
Choose a base branch
from

Conversation

Bchass
Copy link
Contributor

@Bchass Bchass commented Oct 3, 2023

Description

Fixed a bug and switched over to a deque instead of a list

Related Issues

#112

Changes Made

load_proxy_list() doesn't rely on an absolute path anymore, this wasn't working as intended.
rotate_proxy() Switched over to a deque for better efficiency when rotating proxies

Screenshots (if applicable)

N/A

Checklist

Please review and check the following before submitting your pull request:

  • I have followed the project's coding conventions and guidelines
  • I have tested my changes thoroughly
  • I have added/updated relevant documentation
  • My code follows best practices and is easy to understand
  • I have added necessary test cases (if applicable)
  • All existing tests are passing
  • My changes do not introduce any new warnings or errors

Additional Notes

I would like to add test cases for this

@vercel
Copy link

vercel bot commented Oct 3, 2023

Someone is attempting to deploy a commit to a Personal Account owned by @neokd on Vercel.

@neokd first needs to authorize it.

@neokd
Copy link
Owner

neokd commented Oct 3, 2023

Can the proxy be made like an parameter? Like if user set as True then proxy works or else it tries to scrape without proxy.?

@Bchass
Copy link
Contributor Author

Bchass commented Oct 3, 2023

Can the proxy be made like an parameter? Like if user set as True then proxy works or else it tries to scrape without proxy.?

Are you referring to a user-agent a browser would use?

@neokd
Copy link
Owner

neokd commented Oct 3, 2023

No, for now we were thinking that proxy is getting complicated so if we allow user to pass an argument that they want to use scrapper or not.

@Bchass
Copy link
Contributor Author

Bchass commented Oct 3, 2023

No, for now we were thinking that proxy is getting complicated so if we allow user to pass an argument that they want to use scrapper or not.

I see. I'll look into adding that option for the scraper.

@neokd neokd marked this pull request as ready for review October 3, 2023 14:37
Copy link
Contributor Author

@Bchass Bchass left a comment

Choose a reason for hiding this comment

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

@neokd thoughts on this?

@neokd
Copy link
Owner

neokd commented Oct 20, 2023

@Bchass looks good I'll review it fully and merge it. Thinking to publish the scrapper as package on Pypi?

@Bchass
Copy link
Contributor Author

Bchass commented Dec 1, 2023

Is this project being maintained anymore?

@neokd
Copy link
Owner

neokd commented Dec 4, 2023

@Bchass yeah the project is maintained. There was merge conflict so i didn't merge this PR.

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