feat: make impit the default HTTP client#3767
Conversation
|
This makes |
This comment was marked as resolved.
This comment was marked as resolved.
Huh, I don't recall that, but it doesn't sound bad. Do you mean something like a dynamic |
Yes, that's the general idea |
|
I don't remember that either, but it makes sense for sure. I would include the impit client in |
janbuchar
left a comment
There was a problem hiding this comment.
So the general consensus seems to be that we should gracefully handle the case when impit is not installed. Please re-request me once that's done 🙂
|
|
||
| import { createSendRequest } from './send-request.js'; | ||
|
|
||
| class LazyDefaultHttpClient implements BaseHttpClient { |
There was a problem hiding this comment.
This charade is necessary because of AdaptivePlaywrightCrawler, as it doesn't call the async init() on the HTTP-based crawler.
This way, we defer the async import of impit until the actual usage (sendRequest() callsite).
There was a problem hiding this comment.
Oomph. Wouldn't calling init in AdaptivePlaywrightCrawler be less annoying? That being said, the rest of the PR is clean, and this isn't too bad, either. So treat this comment as non-blocking.
There was a problem hiding this comment.
Agreed, but I won't deal w/ AdaptivePlaywrightCrawler internals if I can choose not to 😄
|
|
||
| import { createSendRequest } from './send-request.js'; | ||
|
|
||
| class LazyDefaultHttpClient implements BaseHttpClient { |
There was a problem hiding this comment.
Oomph. Wouldn't calling init in AdaptivePlaywrightCrawler be less annoying? That being said, the rest of the PR is clean, and this isn't too bad, either. So treat this comment as non-blocking.
Makes
@crawlee/impit-clientthe default HTTP client for@crawlee/basic'sBasicCrawler.