refactor: remove browserPoolOptions from BrowserCrawler#3785
refactor: remove browserPoolOptions from BrowserCrawler#3785yuvrajsingh2428 wants to merge 2 commits into
Conversation
yuvrajsingh2428
changed the title
B4nan
added
the
t-tooling
barjin
left a comment
barjin
left a comment
There was a problem hiding this comment.
Thank you @yuvrajsingh2428 for drafting this PR.
I believe the original request feature has design flaws. Let us discuss this before proceeding with work on this PR.
Cheers!
| // Disable the default fingerprint spoofing to avoid conflicts with Camoufox. | ||
| useFingerprints: false, | ||
| }, | ||
| }), |
There was a problem hiding this comment.
This replaces PlaywrightCrawler's implicit BrowserPool with the right browserPlugins: [playwrightLauncher.createBrowserPlugin()] (the custom instance doesn't have any browserPlugins)
I'm afraid the current BrowserPool design (with library-specific launchers) forbids us from dropping the browserPoolOptions from BrowserCrawler. Any user trying to customize BrowserPool options for a crawler would have to pass the right browserPlugins (and make sure these match the BrowserCrawler subclass they are using).
In the end, I'd now rather keep browserPoolOptions / browserPool for simplicity.
cc @janbuchar wdyt ?
There was a problem hiding this comment.
Thanks for the review!
That makes sense. I followed the direction from the issue discussion and assumed that requiring a pre-instantiated browserPool would be preferable to exposing two configuration paths.
I agree that, with the current BrowserPool design, users would also need to know how to provide the correct browserPlugins for the crawler they're using, which leaks implementation details and makes simple customization harder.
I'm happy to adapt the PR based on the outcome of the discussion between you and @janbuchar, or close it if the consensus is to keep supporting both browserPoolOptions and browserPool.
|
@janbuchar @barjin Just following up on the discussion above. I'm happy to either adapt this PR to a revised approach or close it if the consensus is to keep supporting both |
4 participants
Summary
browserPoolOptionsfromBrowserCrawlerbrowserPoolwhen custom browser pool configuration is neededPlaywrightCrawler,PuppeteerCrawler, andStagehandCrawlerto create a defaultBrowserPoolwhen one is not providedbrowserPoolCloses #3728