-
Notifications
You must be signed in to change notification settings - Fork 37
Description
We currently host the representation of values for the --mode
CLI argument in hermeto/core/models/input.py
where it's completely misplaced and should instead be moved to hermeto/interface/cli.py
where other representations just like it reside. However, the moment one tries doing that they'll face a circular import problem in our tests:
ImportError while loading conftest '/tmp/hermeto/tests/unit/conftest.py'.
tests/unit/conftest.py:8: in <module>
from hermeto.core.models.input import Request
hermeto/core/models/input.py:12: in <module>
from hermeto.interface.cli import Mode
hermeto/interface/cli.py:14: in <module>
import hermeto.core.config as config
hermeto/core/config.py:9: in <module>
from hermeto.core.models.input import parse_user_input
E ImportError: cannot import name 'parse_user_input' from partially initialized module 'hermeto.core.models.input' (most likely due to a circular import) (/tmp/hermeto/hermeto/core/models/input.py)
nox > Command pytest --log-level=DEBUG -W ignore::DeprecationWarning --cov=hermeto --cov-config=pyproject.toml --cov-report=term --cov-report=html --cov-report=xml --no-cov-on-fail tests/unit failed with exit code 4
The function causing this is parse_user_input
. The problem is not the function though because trying to extract it to a different module, say utils.py
would just cause a different circular dependency on get_config
. The main culprit here really is how we work with the Config object - we use a set of helper functions inside the same module as the Config class is defined in to hold a global variable as a single instance representing user settings. This is very error prone in OOP and also very non-idiomatic in Python. The proper fix here is to either consider using something like a Singleton pattern for the Config object (which brings other pitfalls we'll have to consider) OR we could pass a reference to the config instance in the Request
object, extract it in the backends and make sure it's passed to the run_cmd
function in utils.py
in order to get rid of the get|set_config
functions in the config.py
module.