Conversation
sdcclient/_monitor.py
Outdated
| return self._request_result(res) | ||
|
|
||
| def convert_scope_string_to_expression(self, scope): | ||
| if scope != None: |
There was a problem hiding this comment.
I would prefer:
if scope is None:
return [True, None]In fact I would return [True, []] so we return always the same type. [] is also False when in conditions.
sdcclient/_monitor.py
Outdated
| if matches == None: | ||
| return [False, 'invalid scope format'] | ||
|
|
||
| is_not = matches.group('not') != None |
There was a problem hiding this comment.
PEP8 recommendations say we should use is None and is not None instead of == None and != None. This is happening many times at the PR, so I guess you are using == to maintain code homogeneity, in that case 👍
There was a problem hiding this comment.
Are they functionally equivalent? Also, this reminds me that we should really add "linting" validation soon...
sdcclient/_monitor.py
Outdated
| elif matches.group('operator') == '!=': | ||
| operator = 'notEquals' | ||
| elif matches.group('operator') == 'contains': | ||
| operator = 'contains' if is_not == False else 'notContains' |
There was a problem hiding this comment.
is_not == False should be changed to not is_not (honestly I would refactor is_not name because of this to something more descriptive.
This is happening more times at the PR. Again, if it is to maintain existing code style, I'm OK.
sdcclient/_monitor.py
Outdated
| elif matches.group('operator') == 'contains': | ||
| operator = 'contains' if is_not == False else 'notContains' | ||
| elif matches.group('operator') == 'starts with': | ||
| operator = 'startsWith' |
There was a problem hiding this comment.
Running away from if/elif we could:
operator_parse_dict = {
'in': 'in' if not is_not else 'notIn',
'=': 'equals',
....
}
operator = operator_parse_dict.get(matches.group('operator'), None)
sdcclient/_monitor.py
Outdated
| list_value = matches.group('value').strip(' ()') | ||
| value_matches = re.findall('[^,]+', list_value) | ||
|
|
||
| if len(value_matches) == 0: |
There was a problem hiding this comment.
PEP8 says we should take the advantage that not value_matches means the same.
2 participants
NOTE: Scope values should be enclosed within single or double quotes. The upcoming API will add more strict validation. For the moment you can still use the old format (e.g.
proc.name = cassandra) but the support will be dropped in the future.It is highly suggested to make sure to enclose all values within either single or double quotes (e.g.
proc.name = "cassandra")