Improve waiting for subprocess-based services to start

[phyrwork]

Use an asyncio.wait_for timeout instead of retrying a fixed number of times.

Also make service state check interval configurable and pass this and timeout options up to subprocess-based Service() implementations.

At timeout raise an ArsenicError instead of a generic Exception so that applications can catch these errors better and decide what to do next.

[phyrwork]

The code is fine, but I guess there is no user-visible change, is there?

The new flag needs to be passed to GeckoDriver/ChromeDriver/WhateverDriver somehow...

IIRC, the main API is start_session perhaps the flag needs to be propagated there? https://arsenic.readthedocs.io/en/latest/reference/index.html#module-arsenic

So get_session and start_session take the Service object where these flags are specified. So as far as I can tell no change is required to these APIs.

You just need to specify start_timeout and/or start_check_interval on the Service object that you were going to use anyways e.g.

driver = services.Chromedriver(start_timeout=60)
browser = browsers.Chrome()
async with get_session(driver, browser) as session:
  session.get('https://some.website')

The documentation does need updating though - I'll do that too.

[dimaqq]

@ojii 👈🏿

[dimaqq]

My 2¢

I'm not a fan of check_interval because it's an implementation detail and once it's in the API, it's not easy to remove (though easy to ignore).

How about making that automatic, for example with exponential back-off:

In [4]: [2 ** i for i in range(-10, 0)]
Out[4]:
[0.0009765625,
 0.001953125,
 0.00390625,
 0.0078125,
 0.015625,
 0.03125,
 0.0625,
 0.125,
 0.25,
 0.5]

In [5]: sum(_)
Out[5]: 0.9990234375

Thus, code could be either:

for i in range(-10, 0):
    xxx
    await asyncio.sleep(start_timeout * 2 ** i)

Or, alternatively:

async def foo():
    for i in range(-10, 9999):
        xxx
        await asyncio.sleep(i)
asyncio.wait_for(foo(), timeout=start_timeout)

[phyrwork]

Sure, I can get behind that.

[dimaqq]

poke

[dimaqq]

Test failed with E aiohttp.client_exceptions.ClientConnectorError: Cannot connect to host localhost:35171 ssl:default [Connect call failed ('127.0.0.1', 35171)]

I can't be sure why, is it CI specific or a general case; specifically, if the browser didn't finish starting yet, perhaps it's not listening on the assigned port and it's not possible to check the WebDriver status.

I wonder if the status check needs to be wrapped in try/except