pelias / openaddresses

Pelias import pipeline for OpenAddresses.

MIT License

53 stars 44 forks source link

feat: add HTTP requests limiting on utils/ to avoid ban #434

Closed severo closed 5 years ago

severo commented 5 years ago

An attempt to fix #433.

orangejulius commented 5 years ago

@severo thanks for this PR.

It looks really good, and I especially like the refactoring to create very sensible objects for each of the downloads.

The limited-request-queue dependency requires Node.js 10 or greater. Pelias currently supports Node.js 8, however we're about to drop support for Node.js 8 as we add support for Node.js 12, so we'll have to wait to merge this until that happens.

severo commented 5 years ago

OK.

Note that I tried to apply the minimum changes, but is there a specific reason to call system tools like curl and unzip, instead of using node modules?

orangejulius commented 5 years ago

Yes, we found using node modules to be extremely slow (Pelias often has to download and decompress very large archive files).

To be honest, I don't think the project goals include supporting other platforms like Windows that would require us to move to native Node.js code for downloads.

severo commented 5 years ago

OK, thanks for the explanation.

severo commented 5 years ago

435 replaces the limited-request-queue dependency by bottleneck, that supports Node.js 8 and is 10 times more popular.