.cook() is not idempotent

[florimondmanca]

• Explain with a simple sentence the expected behavior: it should be possible to call .cook() multiple times without it failing with hard-to-debug error tracebacks.
• Tartiflette version: 0.11.2
• Python version: 3.7.2
• Executed in docker: No
• Is it a regression from a previous versions? No (I think the root cause has always been there?)
• Stack trace: I pushed a failing test in Make .cook() idempotent #255

_______________________________________ ERROR at setup of test_error_handling _______________________________________

ttftt = <tartiflette_starlette.app.TartifletteApp object at 0x105b25668>

    @pytest.fixture(name="client")
    def fixture_client(ttftt: TartifletteApp) -> TestClient:
>       with TestClient(ttftt) as client:

tests/conftest.py:28: 
_ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _
/Users/Florimond/Developer/tartiflette-projects/tartiflette-starlette/venv/lib/python3.7/site-packages/starlette/testclient.py:449: in __enter__
    loop.run_until_complete(self.wait_startup())
/Users/Florimond/.pyenv/versions/3.7.2/lib/python3.7/asyncio/base_events.py:584: in run_until_complete
    return future.result()
/Users/Florimond/Developer/tartiflette-projects/tartiflette-starlette/venv/lib/python3.7/site-packages/starlette/testclient.py:467: in wait_startup
    self.task.result()
/Users/Florimond/Developer/tartiflette-projects/tartiflette-starlette/venv/lib/python3.7/site-packages/starlette/testclient.py:459: in lifespan
    await self.app(scope, self.receive_queue.get, self.send_queue.put)
tartiflette_starlette/app.py:55: in __call__
    await self.lifespan(scope, receive, send)
/Users/Florimond/Developer/tartiflette-projects/tartiflette-starlette/venv/lib/python3.7/site-packages/starlette/routing.py:472: in __call__
    await self.startup()
/Users/Florimond/Developer/tartiflette-projects/tartiflette-starlette/venv/lib/python3.7/site-packages/starlette/routing.py:458: in startup
    await handler()
tartiflette_starlette/app.py:51: in on_startup
    await self.engine.cook()
/Users/Florimond/Developer/tartiflette-projects/tartiflette-starlette/venv/lib/python3.7/site-packages/tartiflette/engine.py:157: in cook
    self._modules, modules_sdl = await _import_modules(modules, schema_name)
/Users/Florimond/Developer/tartiflette-projects/tartiflette-starlette/venv/lib/python3.7/site-packages/tartiflette/engine.py:76: in _import_modules
    module = import_module(module["name"])
_ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _

name = <module 'tests.resolvers' from '/Users/florimond/Developer/tartiflette-projects/tartiflette-starlette/tests/resolvers.py'>
package = None

    def import_module(name, package=None):
        """Import a module.
    
        The 'package' argument is required when performing a relative import. It
        specifies the package to use as the anchor point from which to resolve the
        relative import to an absolute import.
    
        """
        level = 0
>       if name.startswith('.'):
E       AttributeError: module 'tests.resolvers' has no attribute 'startswith'

/Users/Florimond/.pyenv/versions/3.7.2/lib/python3.7/importlib/__init__.py:118: AttributeError

More details

While playing with tartiflette/tartiflette-asgi#32 I ran into issues when .cook() was being called multiple times. (I need to to this because we can't create an Engine multiple times in the same session, and so one single Engine is reused across tests.)

Ultimately, here are the reasons why I think this doesn't work:

• .cook() tries to import modules, although self._modules might already contain the imported module objects from a previous call. Overall I think this (and potentially other issues) can be fixed by adding a simple ._cooked flag and skipping .cook() if it's True.
• .cook() always registers the SDL, even though it may have already been added to the SchemaRegistry before, which fails on second time because the same SDL is used, resulting in conflicting types. (I suppose this is why we use a clean_registry in the engine unit tests.) I don't really have an idea on how to fix this. Should I just clean the registry like we do in the tests before calling .cook() in tartiflette-starlette?

[abusi]

.cook() (and the global process of creating an engine) is meant to be done once.
I think a cooked boolean would help us here (not to cook an already cooked engine).

But you won't be able to do 2 engine with the same SDL if schema_name is the same.

[abusi]

I think the first step of implementing a 'cooked' state is easily doable.

[florimondmanca]

But you won't be able to do 2 engine with the same SDL if schema_name is the same.

That makes sense. But then I'm thinking about people using tartiflette-starlette and writing tests for their Starlette app as I'm doing now. How can we make sure they don't to hack their way through a solution such as resetting SchemaRegistry._schemas so that their tests start from a clean registry? Maybe a public SchemaRegistry.clean() method could help.

[florimondmanca]

FYI I pushed a branch with failing tests in tartiflette/tartiflette-asgi#35.

[abusi]

I see your point, yeah maybe a SchemaRegistry.clean() could also be a cool idea. It would save the time of creating a new Engine/App with a schema_name="test_{x}". (that's what we do in Tartiflette itself for the newest tests, old ones still uses the old "clean_registry" fixture.
A clean method on the registry will also helps us here.

[florimondmanca]

@abusi I pushed fixes in #255 and #258. Feel free to take a look! 👍

[abusi]

Thank you @florimondmanca. I'll make the release tomorrow.

[florimondmanca]

Great, thanks! 🚀