-
Notifications
You must be signed in to change notification settings - Fork 15
Prevent duplicated rule exception when importing rules and filters #87
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
Conversation
jochemvandooren
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Only two lines to fix it, that's nice! 🙌
src/dbt_score/rule_registry.py
Outdated
| module = importlib.import_module(module_name) | ||
| for obj_name in dir(module): | ||
| obj = module.__dict__[obj_name] | ||
| # skip adding objects imported from other modules |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
| # skip adding objects imported from other modules | |
| # Skip adding objects imported from other modules |
🤓
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks, should be fixed now.
tests/rules/imported/__init__.py
Outdated
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I don't fully understand why we need a full rules/imported section for this, as we are just fixing a bug. Wouldn't it be simpler to split the existing examples into rules.py and rule_filters.py, add a filter in rule_filters.py that is imported in rules.py? Then it's also a practical use-case that we expect users to have
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Good point, I have split rules/example.py into rules.py and rule_filters.py, and added a rule filter which is now imported into rules.py. The structure should look better now.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The ordering of objects has changed, because files were renamed:
exampleused to be beforenested,- now
rulesandrule_filterscome afternested.
jochemvandooren
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Nice! 🤩
|
I will give @matthieucan the final say as he worked on the rule registry |
matthieucan
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Tiny comment, but approving. Thanks!
Co-authored-by: Matthieu Caneill <[email protected]>
Fixes #85.
Implementation
tests/rules/example.pyintotests/rules/rules.pyandtests/rules/rule_filters.py.Duplicatedexception is not raised by adding an importedskip_schemaXrule filter.Note
Both rules and rule filters can be imported now, but the existing tests cover only rule filters.