Static Dialect Validation Checks & Tabulator Subclasses #1459
Conversation
- Started working on the custom checks for static file dialects.
- Added fatal error check for custom dialect checks. - Reverted some code. - Fixed typo.
- Moved tabulator related subclasses into new script for reusability between Xloader and Validation.
- More comments for the Tabulator subclasses.
- Renamed canada script.
- Removed unused imports.
- Only enforce dialect if the stream has guessed any.
- Mokey patching the Tabulator stream and csv parser classes.
- Continued monkey patching the tabulator stream class.
- Continued monkey patching the tabulator stream class.
- Limited log messages with a filter class. - Started adding in the exceptions for Xloader.
- Made grandchild class to raise errors to use with Xloader.
- Finalized tabulator classes. - Finalized dialect checks.
# Conflicts: # ckanext/canada/logic.py # ckanext/canada/strings.py ### RESOLVED.
- Removed redundant file check for goodtables, string classes will raise and goodtables will handle. - Minor style fix. - Added comments.
- Started working on custom exception class.
- Used big brain energy to solve the tabulator raising errors.
- Removed unused imports.
|
Okay this is good to go now and has gone through so so so much testing locally. But will test on test ad staging vigorously as well. Did a whole different take on it, as it was almost impossible to really bake this into the CKAN framework (Validation and Xloader hooks). This was mainly because of how the two plugins use Tabulator, as well as all of the private/protected methods and properties the Tabulator library has. So it is mainly all done in the Canada plugin now, monkey-patching the Xloader and Goodtables (Validation) usages of the Tabulator Stream class. Our fork of goodtables has some minor additions to parse the custom errors. The premise of this is that the subclasses won't actually do anything unless the |
| def filter(self, record): | ||
| current_log = (record.module, record.levelno, record.msg) | ||
| if current_log not in self.logged_messages: | ||
| self.logged_messages.append(current_log) |
There was a problem hiding this comment.
this will grow without bounds and searching a list is O(n). You could use a set to make the search O(1) but the growth growth will still be a problem.
Can we store just the last message or last n messages to solve the problem?
|
|
||
|
|
||
| class LimitStreamLogging(Filter): | ||
| logged_messages = [] |
There was a problem hiding this comment.
this is a (global) class variable, better to use an instance variable.
|
|
||
| if guessed_dialect and static_dialect: | ||
| if guessed_delimiter_var in guessed_dialect and guessed_dialect[guessed_delimiter_var] != static_dialect['delimiter']: | ||
| raise TabulatorException('{0}{1}{2}'.format( |
There was a problem hiding this comment.
This is a little odd, you could do something like
| raise TabulatorException('{0}{1}{2}'.format( | |
| raise TabulatorException(''.join([ |
instead, or even more normal, just use +
Requires and Required-by:
open-data/ckanext-xloader#25
open-data/ckanext-validation#13
open-data/goodtables#3