Thank you for docstrings! You have clearly devoted effort to them - consider pointing to an URL of sphinx output.
In tables
, errors
, and
@property
def warnings( self ):
return [] if self._warnings is None else self._warnings
I don't understand why we don't simply return self._warnings
, given that __init__
assigned []
. Also, PEP-8, no blanks around self
. Running flake8
would tidy that up.
I think your recent edit added these four assignments:
class database(object):
_errors = None
_warnings = None
_tables = None
_rows = None
It would be helpful to add a comment mentioning the __new__
vs. __init__
distinction you're going for.
... The value in the dictionary will be another dictionary containing
'error' (an error message stating exactly what the problem is) and 'foreign_key'
(the actual foreign key definition that cannot be fulfilled)
That is a perfectly nice public API, using the flexibility of a dict. A more natural API might return a pair (a tuple) instead of a 2nd dict.
""" Returns False or a string describing a 1215 error message found for the given table and constraint
This is also a nice API, but returning None
would be more pythonic. Elsewhere you have several trinary functions that very nicely return True / False / None - that makes perfect sense.
Pair of typos: "unsigned mistmatch"
The pattern of returning a formatted error string is good, and the one-parameter-per-line formatting is easy to read. There is enough copy-n-paste duplication there to motivate extracting a helper function or two, for some error string fragments.
# if the column the constraint is on doesn't have an index, then 1215
index_found = False
for index in foreign_table.indexes.values():
if index.columns[0] != foreign_column.name:
continue
index_found = True
break
This seems verbose. Consider building a set()
and testing set membership.
This tool seems intended for repeated quick runs over a schema, as it may bail out early upon detecting first error, rather than accumulating a batch of all errors and reporting them together.
In your One Specific Question you wondered whether a certain design decision was the right one. My vote is yes, you made a good choice. It causes the method to become slightly on the long side, but that's fine here. It has clear repetitive logic, and it is good to see all the tests together. At some point a maintainer will likely have to add Yet Another Test for MariaDB 15, and it will be perfectly clear where to start reading, and where to add that Nth test. Good job!