Jc2k/pytest-docker-tools

Dynamic Environment in container definition

alpex8 opened this issue · 9 comments

Hi,

I'm trying to define a container fixture in which I set the environment property via a fixture which yields a dictionary.
I was trying to define it this way:

service_monitor = container(
    name='{service_name_fix.prefix}-monitor1{service_name_fix.suffix}',
    command='python3 /opt/monitor/run.py',
    image='{monitor_image.id}',
    network='{test_net.name}',
    environment='{service_monitor_env}'
)

The Fixture service_machine_monitor_env initially provides a dictionary, but as Fixtures can be overridden the environment can also be adjusted to what is needed for a certain test.

However the fixture is not handled as a dictionary, but as a string. I was trying some things, but it wasn't successful. Is there any way how I can hand my dictionary over to the docker lib via the docker-tools plugin?

Thanks

Jc2k commented

This came up in #8 and the best workaround I had at the time was something like:

@pytest.fixture(scope="module")
def myapp_environment():
    return {
        "CONSUMER_AUTO_OFFSET_RESET": "earliest",
        "CONSUMER_TOPIC": "consume.test_a",
        "PRODUCER_TOPIC": "produce.test_a",
    }

myapp = container(
    image='{myapp_image.id}',
    network='{integration_test_network.name}',
    scope="module",
    ports={
        '5000/tcp': None,
    },
    environment={
        "CONSUMER_AUTO_OFFSET_RESET":  "{myapp_environment['CONSUMER_AUTO_OFFSET_RESET']}",
        "CONSUMER_TOPIC": "{myapp_environment['CONSUMER_TOPIC']}",
        "PRODUCER_TOPIC": "{myapp_environment['PRODUCER_TOPIC']}",
    }
)

Which is only viable if the keys are static. Even so it's a bit boilerplatey.

However it looks like you can pass callables instead of strings to those parameters (see https://github.com/Jc2k/pytest-docker-tools/blob/main/pytest_docker_tools/templates.py#L74 and https://github.com/Jc2k/pytest-docker-tools/blob/main/pytest_docker_tools/templates.py#L104. It looks like those callables have fixture caputring. So this probably works:

service_monitor = container(
    name='{service_name_fix.prefix}-monitor1{service_name_fix.suffix}',
    command='python3 /opt/monitor/run.py',
    image='{monitor_image.id}',
    network='{test_net.name}',
    environment=lambda service_monitor_env: service_monitor_env
)

Probably works.

I'd be interested in exploring if we could detect fixtures and adding special handling like we do for callables though. E.g. Looking at the 2 files i referenced:

class Renderer:
...
    def visit_value(self, val):
        if isinstance(val, str):
            return FixtureFormatter(self.request).format(val)
        elif hasattr(val, "_pytestfixturefunction"):
            return self.request.getfixturevalue(val.__name__)
        elif callable(val):
            return val(
                *[
                    self.request.getfixturevalue(f)
                    for f in inspect.getfullargspec(val)[0]
                ]
            )
        return val

and

class FixtureFinder:
    def visit_value(self, val):
        if isinstance(val, str):
            for literal_text, format_spec, conversion, _ in Formatter().parse(val):
                if format_spec:
                    yield format_spec.split(".")[0].split("[")[0]
        elif hasattr(val, "_pytestfixturefunction"):
            return val.__name__
        elif callable(val):
            yield from inspect.getfullargspec(val)[0]

With those 2 changs in place you can then hopefully just do:

service_monitor = container(
    name='{service_name_fix.prefix}-monitor1{service_name_fix.suffix}',
    command='python3 /opt/monitor/run.py',
    image='{monitor_image.id}',
    network='{test_net.name}',
    environment=service_monitor_env
)

But you'd have to explicitly import the fixture or define it in the same file, which is slightly less magical than usual. But i think overriding it in another module should still work. What do you think? I haven't tried this but i'd be interested to know if it worked for you.

I was aware of the approach in #8. But as you say it's only helpful if the keys are static.

I tried using the lambda expression but
environment=lambda service_monitor_env: service_monitor_env
behaves exactly the same like
environment='{service_monitor_env}'.

Both evaluates to a string.

Did further testing.

Also importing service_monitor_env explicitly and using
environment=service_monitor_env results in a string.

Forget my previous comments. I have another service which is named similar and I mixed up those two :-/

With the adjustments you proposed on classes FixtureFinder and Renderer the docker library is getting dict objects which is what I wanted!
Also when using lambdas it is not required to import the fixture explicitly.

EDIT: I have a folder tests/fixtures and in my primary conftest.py I import everything from that folder. I guess that is why an explicit import isn't necessary in my case.

I guess you suggested a way how this issue can be properly addressed.

I wonder if there is a way to handle this requirement in a way that does not require using explicite imports. In my opinion that would be the most comfortable situation for users.

For example:

service_monitor = container(
    name='{service_name_fix.prefix}-monitor1{service_name_fix.suffix}',
    command='python3 /opt/monitor/run.py',
    image='{monitor_image.id}',
    network='{test_net.name}',
    environment='{service_monitor_env}'
)

The fixture on the environment is given as usual, but it still would be evaluated into a dictionary

Jc2k commented

Complexity aside, I'm not really a fan of that approach. The templates are string templates. I'm reusing the code in python that .format() uses. For some (like me) it would be very surprising for something using f-string/format notation to return a non string.

I have another idea to share when I'm near my desk.

Jc2k commented
class fixtureref:
    def __init__(self, name):
        self._name = name
class FixtureFinder:
    def visit_value(self, val):
        if isinstance(val, str):
            for literal_text, format_spec, conversion, _ in Formatter().parse(val):
                if format_spec:
                    yield format_spec.split(".")[0].split("[")[0]
        elif isinstance(val, fixtureref):
            return val._name
        elif hasattr(val, "_pytestfixturefunction"):
            return val.__name__
        elif callable(val):
            yield from inspect.getfullargspec(val)[0]
class Renderer:
...
    def visit_value(self, val):
        if isinstance(val, str):
            return FixtureFormatter(self.request).format(val)
        elif isinstance(val, fixtureref):
            return self.request.getfixturevalue(val._name)
        elif hasattr(val, "_pytestfixturefunction"):
            return self.request.getfixturevalue(val.__name__)
        elif callable(val):
            return val(
                *[
                    self.request.getfixturevalue(f)
                    for f in inspect.getfullargspec(val)[0]
                ]
            )
        return val
service_monitor = container(
    name='{service_name_fix.prefix}-monitor1{service_name_fix.suffix}',
    command='python3 /opt/monitor/run.py',
    image='{monitor_image.id}',
    network='{test_net.name}',
    environment=fixtureref('service_monitor_env')
)

You'd still need to import fixtureref, but you could do it when you were importing container so its not too arduous. It's also explicit that you are passing the full fixture straight through.

I would like to start teaching this code base about type hints, and I have no idea how to handle this case with type hints. A sprinkling of typing.Any i suspect.

Jc2k commented

BTW instead of hasattr, i'd probably switch to _pytest.fixtures.getfixturemarker, which returns a FixtureFunctionMarker if called on a fixture, otherwise None.

Jc2k commented

Just pushed 3.1.0 with your PR. The gerbils should push it to pypi shortly. Cheers!