fix for issue/217-Specifying-asterisk-as-query-causes-error #250
Conversation
1 similar comment
|
@Code0x58 , @jorgebastida get back if you see anything wrong with this. Else it's been waiting to be merged for a while now. |
| return s | ||
|
|
||
|
|
||
| def seconds_since_epoch(datetime_text): |
There was a problem hiding this comment.
Milliseconds since epoch?
Good change 👍
| except Exception as e: | ||
| raise exceptions.InvalidPythonRegularExpressionError('Log stream name pattern', s) | ||
|
|
||
| return s |
There was a problem hiding this comment.
Usually the return value/what is passed around after handling the CLI would be the compiled regex - it may also speed up the code if it the code using it is called more than once, as the cache use isn't free.
|
|
||
| - Expressions can be regular expressions or the wildcard ``ALL`` if you want any and don't want to type ``.*``. | ||
| - STREAM_EXPRESSION is a python regular expression accepted by ``re.compile()`` `described here <https://docs.python.org/3/library/re.html#regular-expression-syntax>`_. Expression ``ALL`` is reserved and is same as ``'.*'``. Remember to quote/escape shell special characters to ensure they are not gobbled up by shell variable expansion. E.g. ``'2014-04.*'`` instead of ``2014-04.*`` |
There was a problem hiding this comment.
This doesn't mention how the regex is used. It is used in .match(…), rather than say .fullmatch(…) (which I might prefer but have no strong opinion†) ,or .search(…) (which it sounds like that's how people imagine it being used).
† changing from .match(…) would be a breaking change (dropping the ^ but still using .match(…) is not as it acts exactly the same) so should be a major version change+in release notes
Fix for #217
Added fix for #180 as well.