Accept `wait_for_callable` timeout value as a parameter in the container fixture factory.

[recursive-beast]

Currently, the container factory is only using the default timeout value, which is 30 secs as specified in the wait_for_callable signature.

Using this conftest.py , I get ContainerNotReady exceptions and all tests fail (sometimes some off them pass if the container becomes ready before the timeout occurs) :

import socket

from pytest_docker_tools import container, wrappers


class RabbitMQContainer(wrappers.Container):
    def ready(self):
        """Called until it returns True"""
        if super().ready():
            return self.is_port_open(5672)
        return False

    def is_port_open(self, port: int) -> bool:
        """Check if the port is open in the container."""
        s = socket.socket(socket.AF_INET, socket.SOCK_STREAM)
        s.settimeout(1)
        try:
            s.connect((self.ips.primary, int(port)))
            s.shutdown(socket.SHUT_RDWR)
            return True
        except:
            return False
        finally:
            s.close()


rabbitmq = container(
    image="rabbitmq:management-alpine",
    wrapper_class=RabbitMQContainer,
)

I tried with the rabbitmq:alpine , all tests pass since it starts faster than rabbitmq:management-alpine

[Jc2k]

It's been a while since i looked at this code but I'd take a PR that did a kwargs.pop('timeout', 30) in container() - if you are in there playing with the code it might be quickest if you tried it?

[recursive-beast]

@Jc2k I have created the pull request, just thought that i should remind you just in case you didn't see a notification 😄

[Jc2k]

Thanks @soufyakoub - i'll try and remember how to do a release to pypi, its been a while :-)

[Jc2k]

0.2.1 is out - let me know if it works for you!

[recursive-beast]

@Jc2k

It works but i made a really bad mistake in the code that raises TypeError in the raw container :

TypeError: run() got an unexpected keyword argument 'timeout'

How do i approach this ? a new branch or a new commit on the wait_for_callable_timeout ?

[Jc2k]

You just need to pop from kwargs earlier. Could you try something like this and if it works do another PR:

@fixture_factory()
def container(request, docker_client, wrapper_class, **kwargs):
    ''' Docker container: image={image} '''

    timeout = kwargs.pop('timeout', 30)

    kwargs.update({'detach': True})

    raw_container = docker_client.containers.run(**kwargs)
    request.addfinalizer(lambda: raw_container.remove(force=True) and raw_container.wait(timeout=10))

    wrapper_class = wrapper_class or Container
    container = wrapper_class(raw_container)

    try:
        wait_for_callable('Waiting for container to be ready', container.ready, timeout)
    except TimeoutError:
        raise ContainerNotReady(container, 'Timeout while waiting for container to be ready')

    return container

[recursive-beast]

@Jc2k I submitted the pull request.
My first contribution to a real open source project had to be a mess up LOL, sorry about that.

Maby later, but should i make timeout a positional argument with a default value of 30 and document it in the README.md ?

[Jc2k]

No worries about the error! I should have tested it properly - but didn't have time.

it would be really great if you could update the docs indeed!

[recursive-beast]

Should i open a new issue for it ?

[recursive-beast]

@Jc2k It looks like you forgot to update the package in the python package repository, i tried reinstalling it from my end , and i received the version with the TypeError.

[Jc2k]

I'm away from the machine with the packaging tools on, was going to release a bit later.

[recursive-beast]

Oh ok, then i should start working on the documentation part, that way you release only once

[Jc2k]

Will try and do the release this evening or first thing tomorrow morning

[recursive-beast]

OK, take your time. That's what i learned from this contribution 😆

[recursive-beast]

Hello @Jc2k,
I'm here to remind you about republishing your package due to the issue I introduced before, no one wants to install your package and then get an error when trying to create a container fixture.

[Jc2k]

Oops! Thanks for the reminder! Had totally 100% forgotten 😊 Released now. And thanks again for your help with this.