chore: log multiple errors #14064
chore: log multiple errors #14064
Conversation
Codecov Report
@@ Coverage Diff @@
## master #14064 +/- ##
=======================================
Coverage 66.40% 66.40%
=======================================
Files 1641 1641
Lines 63518 63520 +2
Branches 6422 6422
=======================================
+ Hits 42176 42181 +5
+ Misses 19681 19678 -3
Partials 1661 1661
Flags with carried forward coverage won't be shown. Click here to find out more.
Continue to review full report at Codecov.
|
| action=f"db_connection_failed.{exception.__class__.__name__}" | ||
| action="db_connection_failed.{}.{}".format( | ||
| exception.__class__.__name__, | ||
| ".".join(exception.get_list_classnames()), |
There was a problem hiding this comment.
Should we sort the list of class names, so that the exceptions with the same classes get grouped together?
There was a problem hiding this comment.
sounds good.. is it more or less helpful to see both errors if they are identical? How about filtering out duplicates?
There was a problem hiding this comment.
I think filtering duplicates make sense.
| ".".join(exception.get_list_classnames()), | |
| ".".join(sorted(exception.get_list_classnames())), |
There was a problem hiding this comment.
if we filter out duplicates, do we still need to sort?
There was a problem hiding this comment.
Per discussion, I added the sort to the get_list_classnames method above.
superset/commands/exceptions.py
Outdated
| def get_list_classnames(self) -> List[str]: | ||
| return list( | ||
| dict.fromkeys([ex.__class__.__name__ for ex in self._invalid_exceptions]) | ||
| ) | ||
|
|
There was a problem hiding this comment.
| def get_list_classnames(self) -> List[str]: | |
| return list( | |
| dict.fromkeys([ex.__class__.__name__ for ex in self._invalid_exceptions]) | |
| ) | |
| def get_list_classnames(self) -> List[str]: | |
| return list(set(ex.__class__.__name__ for ex in self._invalid_exceptions)) |
There was a problem hiding this comment.
will a set preserve ordering?
| action=f"db_connection_failed.{exception.__class__.__name__}" | ||
| action="db_connection_failed.{}.{}".format( | ||
| exception.__class__.__name__, | ||
| ".".join(exception.get_list_classnames()), |
There was a problem hiding this comment.
I think filtering duplicates make sense.
| ".".join(exception.get_list_classnames()), | |
| ".".join(sorted(exception.get_list_classnames())), |
eschutho
force-pushed
the
elizabeth/log-multiple-db-errors
branch
2 times, most recently
from
bd41db0
to
d5c6329
Compare
eschutho
force-pushed
the
elizabeth/log-multiple-db-errors
branch
from
d5c6329
to
1d29e9f
Compare
Oh my goodness.. haha. :) |
* log all errors from db create * return unique set of errors * sort set for exceptions list * run black (cherry picked from commit c143b37)
mistercrunch
added
🍒 1.5.2
🍒 1.5.3
🏷️ bot
SUMMARY
The database create command can have multiple errors, but when they are logged, we only see the
DatabaseInvalidError. This change passes all of the errors to the loggerTEST PLAN
unit test added
ADDITIONAL INFORMATION