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

Replace request with fetch #528

Merged
merged 5 commits into from
Dec 14, 2022
Merged

Replace request with fetch #528

merged 5 commits into from
Dec 14, 2022

Conversation

antonok-edm
Copy link
Collaborator

The request package is deprecated and unmaintained. The same functionality can be achieved with the built-in fetch API, so I removed the dependency and updated the code accordingly.

Progress towards #380.

@antonok-edm
Copy link
Collaborator Author

This should also fix the issue with the Estonian regional adblock list I noted in #380 (comment)

Copy link
Member

@diracdeltas diracdeltas left a comment

Choose a reason for hiding this comment

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

@antonok-edm
Copy link
Collaborator Author

Also of note: we'd need to run this in Node 18 (Readable.fromWeb is only available starting in Node 17).

@antonok-edm
Copy link
Collaborator Author

@diracdeltas looks like a Python 2->3 error.

npm ERR! npm ERR! gyp ERR! stack Error: Python executable "/usr/bin/python" is v3.10.6, which is not supported by gyp.
npm ERR! npm ERR! gyp ERR! stack You can pass the --python switch to point to Python >= v2.5.0 & < 3.0.0.

I'm not sure what's causing that specifically, but I assume Node 18 has a better chance of handling Python 3 correctly, too.

@mihaiplesa mihaiplesa force-pushed the replace-request-with-fetch branch from 4199e1f to 4427f0a Compare December 8, 2022 17:34
@antonok-edm
Copy link
Collaborator Author

I see that fetch is also just not supported at all in Node 16 either, so even more reason to upgrade to 18.

@mihaiplesa mihaiplesa force-pushed the replace-request-with-fetch branch from 4427f0a to f0a6ddf Compare December 14, 2022 07:12
@mihaiplesa
Copy link
Contributor

This is good to go now.

@antonok-edm antonok-edm merged commit 94885d7 into master Dec 14, 2022
@antonok-edm antonok-edm deleted the replace-request-with-fetch branch December 14, 2022 17:11
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.

3 participants