inmanta / inmanta-core

Inmanta is an automation and orchestration tool
https://inmanta.com
Apache License 2.0
27 stars 7 forks source link

errors in parse of method paths are not reported correctly #4931

Open FloLey opened 2 years ago

FloLey commented 2 years ago

While working on adding and endpoint API I made a mistake in the path of my endpoint and forgot the leading slash. This took a while (and the help of Arnaud) to debug as the error that was reported didn't gave any indication about the issue and didn't seem to be related:

E   Exception: Method lsm_service_catalog_list already has a method definition for api version 1
ERROR: InvocationError for command /home/jenkins/workspace/dpoint-export-service-definition/project/inmanta-lsm/.tox/py39/bin/py.test --cov=inmanta_ext.lsm --cov=inmanta_lsm --cov-report term --cov-report xml --junitxml=junit-py39.xml -vvv tests/ (exited with code 4)

This is strange as apparently a check exist for this on inmanta-core: https://github.com/inmanta/inmanta-core/blob/255983bdc4a4985c38cb1f81c2ccc458736b6ba1/src/inmanta/protocol/common.py#L298

arnaudsjs commented 2 years ago

It turns out that this issue is caused by the following piece of code in the click_plugins/core.py file:

def with_plugins(plugins):

    """
    A decorator to register external CLI commands to an instance of
    `click.Group()`.

    Parameters
    ----------
    plugins : iter
        An iterable producing one `pkg_resources.EntryPoint()` per iteration.
    attrs : **kwargs, optional
        Additional keyword arguments for instantiating `click.Group()`.

    Returns
    -------
    click.Group()
    """

    def decorator(group):
        if not isinstance(group, click.Group):
            raise TypeError("Plugins can only be attached to an instance of click.Group()")

        for entry_point in plugins or ():
            try:
                group.add_command(entry_point.load())
            except Exception:
                # Catch this so a busted plugin doesn't take down the CLI.
                # Handled by registering a dummy command that does nothing
                # other than explain the error.
                group.add_command(BrokenCommand(entry_point.name))

        return group

    return decorator

Click ignores all exceptions while it is loading its commands. The following sequence of steps cause the above-mentioned issue:

  1. The conftest.py file from pytest-inmanta-extensions perform import inmanta.main. This makes click discover all its entrypoints.
  2. The methods.py from inmanta-lsm gets loaded and an exception is raised regarding the fact that the leading slash is missing in the path of the endpoint (See: https://github.com/inmanta/inmanta-core/blob/255983bdc4a4985c38cb1f81c2ccc458736b6ba1/src/inmanta/protocol/common.py#L298)
  3. Click ignores the exceptions
  4. When the methods.py gets loaded a second time, this time from inmanta-lsm/tests/conftest.py, an exceptions is raised because some methods were already loaded during the first (partial) load of the methods.py file.
  5. The developer incorrectly gets an error message regarding a duplicate endpoint being defined in the code.

It's not immediately clear to me what the best way is to make sure that developers get the right error message. This requires further investigation.