-
Notifications
You must be signed in to change notification settings - Fork 2
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
Intercept KeyError on navigation straight to the metadata tab without adding a config. #95
base: main
Are you sure you want to change the base?
Conversation
Solves the first part (but not all of) of #83. If no config found, then just show a message explaining that there's no config loaded and return. Also the function didn't explicitly return if ``metadata_output_children`` so also fix that.
@@ -373,6 +391,7 @@ def create_metadata_table_and_buttons( | |||
generate_yaml_tooltip, | |||
] | |||
) | |||
return None |
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.
Not sure if we ever get into this situation? Should I throw an error? Or return an empty Div?
Codecov Report
@@ Coverage Diff @@
## main #95 +/- ##
==========================================
+ Coverage 29.81% 39.17% +9.36%
==========================================
Files 12 13 +1
Lines 691 702 +11
==========================================
+ Hits 206 275 +69
+ Misses 485 427 -58
📣 We’re building smart automated test selection to slash your CI/CD build times. Learn more |
Solves the first part of
If no config is found, then show a message explaining that there's no config loaded and return. Also, the function didn't explicitly return if
metadata_output_children
so I also fix that.The eagle-eyed amongst you might notice that this is testless 🤢#WhatAboutAllOfYouTDDEvangelismSam
?. But this will be covered by a tweak to one of the files introduced in #82 (which should probably go in first but not mandatory).Edit: Now ready for review (sorry for the noise). Tested and looking good. I also fixed an error on the ROI page (which now also loads without error if no config).
The dashboard is still an
XFAIL
and is a bit more complicated so should go in a separate PR.