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

Regression in class_mappings after refactoring #84

Open
uberspot opened this issue Apr 30, 2019 · 4 comments
Open

Regression in class_mappings after refactoring #84

uberspot opened this issue Apr 30, 2019 · 4 comments

Comments

@uberspot
Copy link

uberspot commented Apr 30, 2019

I'm not 100% sure that's the issue, but after updating to reclass 1.6.0 the following in reclass-config.yml stopped working:

class_mappings:
  - temp/*        type.test 

and it did work before.
I also tested:

class_mappings:
  - \*        type.test

and it does work.

@uberspot
Copy link
Author

uberspot commented Apr 30, 2019

After searching a bit and trying different commits from reclass I think the bug is introduced in #68 specifically commit d2762b0

@uberspot
Copy link
Author

@epcim gentle bump ^ :)

@AndrewPickford
Copy link
Collaborator

The proposed fix adds back the line name = os.path.splitext(relpath)[0] from the get_node method in yaml_fs. This was removed as part of a patch to fix the use of relative class names. But relative class name loading still works with the line added back in, so there does not seem to be a problem there.

By default node names are just the name of the node file less the .yml extension without any path information so it would be strange for the class_mappings to then wildcard match based on the path of the node file. Having path based wildcards work initially may have been unintentional.

However I don't see why path based wild carding can't be supported. My proposal is to add a path parameter to the Entity class, set it to the value of os.path.splitext(relpath)[0] (and add something equivalent to the get_node method in yaml_git). An option for class_mappings to wildcard either the node name or the path could then be added. It's a small amount of code so if this is acceptable I can code it up.

@AndrewPickford
Copy link
Collaborator

Added pull request #93, with the fix to allow class mappings to match either the node name or node path depending on a configuration option.

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

No branches or pull requests

2 participants