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

Fix Placeholder parsing and improve os_specific #10

Merged
merged 3 commits into from
Aug 23, 2022

Conversation

MHendricks
Copy link
Member

Checklist

  • I have read the CONTRIBUTING.md document
  • I formatted my changes with black
  • I linted my changes with flake8
  • I have added documentation regarding my changes where necessary
  • Any pre-existing tests continue to pass
  • Additional tests were made covering my changes

Types of Changes

  • Bugfix (change that fixes an issue)
  • New Feature (change that adds functionality)
  • Documentation Update (if none of the other choices apply)

Proposed Changes

Fixes exception raised if you use a uri that would hit a Placeholder object.

For the given configs:

$ hab dump . -t f
-------------------- Configs ---------------------
app
    hab.parsers.placeholder.Placeholder('app')
    |-- hab.parsers.config.Config('app/houdini19_py2')
    +-- hab.parsers.config.Config('app/maya2020')
default
    hab.parsers.config.Config('default')

trying to access a non-defined config in the same tree as app results in a error:

$ hab dump app/invalid
Traceback (most recent call last):
  File "C:\Program Files\Python39\lib\runpy.py", line 197, in _run_module_as_main
    return _run_code(code, main_globals, None,
  ...
  File "C:\blur\dev\hab_\hab\parsers\flat_config.py", line 26, in _collect_values
    logger.debug("Loading node: {} inherits: {}".format(node.name, node.inherits))
AttributeError: 'Placeholder' object has no attribute 'inherits'

This fixes the error by enabling inherits for all Placeholder objects, and fixing a bug where it wouldn't fall back to default in that case. That code wasn't being tested, but is now.

Also added tests for the os_specific feature on site json files and for environment variable definitions.

Fixed bug where default was not processed by FlatConfig.
Added missing coverage tests for FlatConfig.
Move platform lookup to utils and hab_base passes it to MergeDict
@MHendricks MHendricks merged commit 0677dad into blurstudio:main Aug 23, 2022
@MHendricks MHendricks deleted the placeholder branch August 23, 2022 22:29
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

1 participant