Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Project templating using TemplatedConfigLoader raises ValidationError in 0.18.0 #1402

Closed
patrikhardin opened this issue Apr 1, 2022 · 4 comments · Fixed by #1467
Closed
Assignees
Labels
Issue: Bug Report 🐞 Bug that needs to be fixed Stage: Technical Design 🎨 Ticket needs to undergo technical design before implementation

Comments

@patrikhardin
Copy link
Contributor

Description

When following the guide for Template configuration, Kedro 0.18.0 raises ValidationError as kedro.config.templated_config.TemplatedConfigLoader is not recognized as a subclass of kedro.config.config.ConfigLoader.

Context

This bug hinders me from using the TemplatedConfigLoader, according to the documentation.

Steps to Reproduce

  1. Create new Kedro project
kedro new --starter=pandas-iris
cd new-kedro-project
  1. Create and activate venv. Install requirements.
python -m venv my_venv
source my_venv/bin/activate
pip install -r src/requirements.txt
  1. Open the settings.py file.
nano src/new_kedro_project/settings.py

Uncomment lines 28,29,31,32,33 as suggested in the guide. Expected config:

# Class that manages how configuration is loaded.
from kedro.config import TemplatedConfigLoader
CONFIG_LOADER_CLASS = TemplatedConfigLoader
# Keyword arguments to pass to the `CONFIG_LOADER_CLASS` constructor.
CONFIG_LOADER_ARGS = {
    "globals_pattern": "*globals.yml",
}
  1. Run the default pipeline
kedro run

Expected Result

Defeault pipeline should run with global config keys overwritten.

Actual Result

ValidationError is raised.

Traceback (most recent call last):
  File "/usr/local/bin/kedro", line 8, in <module>
    sys.exit(main())
  File "/usr/local/lib/python3.9/site-packages/kedro/framework/cli/cli.py", line 205, in main
    cli_collection = KedroCLI(project_path=Path.cwd())
  File "/usr/local/lib/python3.9/site-packages/kedro/framework/cli/cli.py", line 114, in __init__
    self._metadata = bootstrap_project(project_path)
  File "/usr/local/lib/python3.9/site-packages/kedro/framework/startup.py", line 155, in bootstrap_project
    configure_project(metadata.package_name)
  File "/usr/local/lib/python3.9/site-packages/kedro/framework/project/__init__.py", line 166, in configure_project
    settings.configure(settings_module)
  File "/usr/local/lib/python3.9/site-packages/dynaconf/base.py", line 182, in configure
    self._wrapped = Settings(settings_module=settings_module, **kwargs)
  File "/usr/local/lib/python3.9/site-packages/dynaconf/base.py", line 235, in __init__
    self.validators.validate(
  File "/usr/local/lib/python3.9/site-packages/dynaconf/validator.py", line 417, in validate
    validator.validate(self.settings, only=only, exclude=exclude)
  File "/usr/local/lib/python3.9/site-packages/kedro/framework/project/__init__.py", line 35, in validate
    raise ValidationError(
dynaconf.validator.ValidationError: Invalid value `kedro.config.templated_config.TemplatedConfigLoader` received for setting `CONFIG_LOADER_CLASS`. It must be a subclass of `kedro.config.config.ConfigLoader`.

Your Environment

  • Kedro version used (pip show kedro or kedro -V): 0.18.0
  • Python version used (python -V): 3.9.10
  • Operating system and version: macOS 12.3
@noklam noklam added the Issue: Bug Report 🐞 Bug that needs to be fixed label Apr 1, 2022
@datajoely
Copy link
Contributor

Hi @patrikhardin thanks for raising this - we're looking into it. As part of 0.18.x there is a new AbstractConfigLoader.

@antonymilne
Copy link
Contributor

antonymilne commented Apr 1, 2022

Thanks @patrikhardin. This is indeed a mistake where we should have specified AbstractConfigLoader instead here. Not quite sure how this got through our tests, sorry about that Actually it's not such a simple fix... see next message.

For the time being, here's a super hacky fix. Use the following in settings.py:

# Class that manages how configuration is loaded.
from kedro.config import TemplatedConfigLoader, ConfigLoader
ConfigLoader.__subclasses__ = lambda: [TemplatedConfigLoader]
CONFIG_LOADER_CLASS = TemplatedConfigLoader
# Keyword arguments to pass to the `CONFIG_LOADER_CLASS` constructor.
CONFIG_LOADER_ARGS = {
    "globals_pattern": "*globals.yml",
}

This will trick the validation code into thinking that TemplatedConfigLoader is a subclass of ConfigLoader.

@antonymilne
Copy link
Contributor

antonymilne commented Apr 1, 2022

Here's a new version of test_settings.py that picks up on this and also does some more thorough testing of other settings: https://gist.github.com/AntonyMilneQB/50159cbbde40399fc6d9094fef3b0fee

The problem is that it's not immediately possible to get both tests passing simultaneously. Changing ConfigLoader to AbstractConfigLoader doesn't work because then it will change the default value for CONFIG_LOADER_CLASS to AbstractConfigLoader, which isn't what we want. Basically _IsSubclassValidator is designed to treat default as the thing that must be the superclass also. We need to modify this somehow:

  • make it so we can specify default and the superclass independently?
  • change the validation so that it just checks that the class specified shares some non-trivial parent class with default (sounds less rigorous but maybe easier to implement)?
  • just make the validation much slacker so we don't check for subclasses at all? Easy to do just by changing _IsSubclassValidator to Validator

We were doing the last of these options before #1064 which changed Validator to _IsSubclassValidator and inadvertently introduced this bug. I think we need to work out out whether we actually care about checking for subclass validation.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Issue: Bug Report 🐞 Bug that needs to be fixed Stage: Technical Design 🎨 Ticket needs to undergo technical design before implementation
Projects
None yet
Development

Successfully merging a pull request may close this issue.

4 participants