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

"urllib" to "requests" library change #182

Closed
wants to merge 7 commits into from
Closed

Conversation

MUCCHU
Copy link

@MUCCHU MUCCHU commented Dec 26, 2023

Fixes #180

Migrated the project from urllib to requests library. And also modified the import of OrderedDict and defaultdict from the collections framework. @benoit74 Kindly review my PR.

@MUCCHU
Copy link
Author

MUCCHU commented Dec 26, 2023

While installing the requirements.txt file I faced issues when it was trying to install zimscraperlib>=1.3.6,<1.4. On installing without specifying any version, it worked flawlessly and installed version 3.2.0. Should I alter the version of zimscaperlib in the requirements file?

@kelson42 kelson42 changed the title Fix #180 urllib to request library change Dec 26, 2023
@kelson42 kelson42 changed the title urllib to request library change "urllib" to "requests" library change Dec 26, 2023
@benoit74
Copy link
Collaborator

Thank you for this contribution again, I'm unfortunately currently on holidays so I won't have time to review this in details until next week.

When you say that "it worked flawlessly", what did you tested specifically ? This scraper is known to have a major issue currently (#175) so I doubt you could really have tested something. And upgrading two major versions is not expected to go smoothly without any code change.

Regarding the issue you faced with zimscraperlib>=1.3.6,<1.4, which Python version did you used? This scraper is still on Python 3.8 and I can imagine such old zimscraperlib version might have issues with more recent Python versions (while obviously more recent zimscraperlib don't).

@joe-rabbit
Copy link

python 3.10.12 . I faced a similar issue

@benoit74
Copy link
Collaborator

benoit74 commented Jan 8, 2024

Please use the proper Python version. If scraper is still on 3.8, there is 99% chance it won't work with 3.10

Or if you feel like you could do this, upgrade to more recent Python. We aim to support only one Python version per scraper, currently our target is 3.11 (zimscraperlib does not yet support 3.12).

@benoit74
Copy link
Collaborator

benoit74 commented Jan 9, 2024

To be honest, I think you shouldn't spend too much time on this issue, the scraper is not working anymore and complex, you won't be able to really test your change. Taking an issue from youtube / ted / freecodecamp / kolibri / zimfarm would be more appropriate.

@benoit74
Copy link
Collaborator

benoit74 commented Feb 1, 2024

Closing this for now, feel free to reopen if needed

@benoit74 benoit74 closed this Feb 1, 2024
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.

Migrate from urllib to requests
3 participants