I had to empty commit twice in order to make the tests rerun and all pass. They seem a bit flaky (every commit failing a different tests). No idea if this is expected or not? Also no idea if my change is causing this, it doesn't feel like it's supposed to? WDYT?

I had to empty commit twice in order to make the tests rerun and all pass. They seem a bit flaky (every commit failing a different tests). No idea if this is expected or not? Also no idea if my change is causing this, it doesn't feel like it's supposed to? WDYT?

Sorry for that. I have checked and both failures seems to be somehow related to BrowserPool, I will take a look if I can figure out the root cause. (Btw. I do not think it is related to your change.)

unfortunately it does not work if you first create a browser pool:

crawlee.errors.ServiceConflictError: Service Configuration is already in use

because BrowserPool.with_default_plugin calls PlaywrightBrowserPlugin.__init__ that calls config = service_locator.get_configuration(), this "taints" the config and sets _configuration_was_retrieved

Thanks for the heads up @sherpya! Do you have an executable code snippet that would reproduce the issue?

Thanks for the heads up @sherpya! Do you have an executable code snippet that would reproduce the issue?

import os
from datetime import timedelta

from crawlee import ConcurrencySettings, service_locator
from crawlee.browsers import BrowserPool
from crawlee.configuration import Configuration
from crawlee.crawlers import PlaywrightCrawler

from .request import RequestType, make_request
from .routes import router
from ..settings import settings

__all__ = ['WACrawler']


class WACrawler(PlaywrightCrawler):
    def __init__(self, **kwargs):
        #service_locator.set_configuration(
        #    Configuration(persist_storage=False, default_request_queue_id=str(os.getpid())))
        browser_pool = BrowserPool.with_default_plugin(
            headless=kwargs.pop('headless', None),
            browser_type='chromium',
            fingerprint_generator=None,
            browser_launch_options=kwargs.pop('browser_launch_options', None),
            browser_new_context_options=kwargs.pop('browser_new_context_options', None),
            use_incognito_pages=False,
            operation_timeout=timedelta(milliseconds=settings.TIMEOUT),
        )
        concurrency_settings = ConcurrencySettings(max_concurrency=1)
        super().__init__(browser_pool=browser_pool,
                         concurrency_settings=concurrency_settings,
                         request_handler=router,
                         request_handler_timeout=timedelta(milliseconds=settings.TIMEOUT),
                         configuration=Configuration(persist_storage=False, default_request_queue_id=str(os.getpid())),
                         **kwargs)

The commented code is what I use right now

I think that you should be able to circumvent this if you set the configuration before the creation of the BrowserPool and then not passing Configuration to the crawler directly, as it will take it from the service_locator.

...
service_locator.set_configuration(Configuration(persist_storage=False, default_request_queue_id=str(os.getpid())))
browser_pool = BrowserPool...

I think that you should be able to circumvent this if you set the configuration before the creation of the BrowserPool and then not passing Configuration to the crawler directly, as it will take it from the service_locator.

...
service_locator.set_configuration(Configuration(persist_storage=False, default_request_queue_id=str(os.getpid())))
browser_pool = BrowserPool...

yes, is what I do, look at the commented code above

yes, is what I do, look at the commented code above

That seems to work, so have you found any limitations to the mentioned approach?

yes, is what I do, look at the commented code above

That seems to work, so have you found any limitations to the mentioned approach?

no, it works well, but not in the way this merge should make it work