-
Notifications
You must be signed in to change notification settings - Fork 0
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
Update Concept and Scheme models #127
Conversation
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.
lgtm! thanks!
|
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.
+1 tested. Few things noticed along the way that can be follow-ups:
- make etl_modules an explicit package, so that it's measured by coverage
- use CommandError
- catch CommandError and surface the error message in the GUI, but may want to check with @chiatt
1c5688e
to
47151b4
Compare
@robgaston coverage was previously passing because Lingo ETL Modules was not a package. Given the blocking nature of these model changes for other work in flight, I created a follow-up issue #131. Would you like to see that addressed in this PR or okay to do the follow up? |
@johnatawnclementawn it's fine to address the coverage in the follow up ticket |
Resolves #123
This will break the test data - the RDM -> Lingo migration will need to be re-run, and likely updated based on new model configurations.