refactor utils, add tests, move exceptions into separate module #264
Conversation
|
@demitryfly tests failed |
simple_ddl_parser/utils.py
Outdated
| """ | ||
| Remove the parentheses from the given list | ||
|
|
||
| Warn: p_list may contain unhashable types for some unexplored reasons |
There was a problem hiding this comment.
It's better to find the reason why p_list may contain dict. Is it expected?
There was a problem hiding this comment.
yes, it is expected, p_list contains results of parsing statements, usually it something like {'column': name, 'unique': True}, so p_list - always list, but elements of this list can be dicts
There was a problem hiding this comment.
Ok, I rewrote this docstring then.
demitryfly
changed the title
demitryfly
changed the title
simple_ddl_parser/parser.py
Outdated
| @@ -348,7 +346,7 @@ def run( | |||
| Dict == one entity from ddl - one table or sequence or type. | |||
| """ | |||
| if output_mode not in dialect_by_name: | |||
| raise SimpleDDLParserException( | |||
There was a problem hiding this comment.
I am not sure, if someone uses this exception in their code (to catch it in try-except, for instance). If, it is possible, then it is more correct to make an alias and deprecate it officially.
There was a problem hiding this comment.
Do not rename exceptions, I didn’t merge it because of those changes. Sorry for long delay with review. Didn’t have any time
There was a problem hiding this comment.
Fixed.
Let me know, it there are something else to be fixed :)
There was a problem hiding this comment.
I think, there is no reason to exist both SimpleDDLParserException and DDLParserError exception type. Only backward compatibility. (If I understood correctly)
find_first_unpair_closed_parandremove_parimplementationsSimpleDDLParserException(it's better to discuss, backward compatibility may be broken)