search

openzim / openedx

Open edX (to zim) scraper
GNU General Public License v3.0
8 stars 7 forks source link

"urllib" to "requests" library change #182

Closed MUCCHU closed 10 months ago

MUCCHU commented 11 months ago

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 commented 11 months ago

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?

benoit74 commented 11 months ago

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 (https://github.com/openzim/openedx/issues/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 commented 11 months ago

python 3.10.12 . I faced a similar issue

benoit74 commented 10 months ago

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 commented 10 months ago

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 commented 10 months ago

Closing this for now, feel free to reopen if needed