jenkins-bot has submitted this change and it was merged. (
https://gerrit.wikimedia.org/r/363748 )
Change subject: Query and grammar: add support for aliases
......................................................................
Query and grammar: add support for aliases
- allow aliases of the form A:alias_name into the grammar.
- automatically replace recursively all the aliases directly in the
QueryBuilder, to make it completely transparent for the backends.
- In this first iteration the Direct backend do not support aliases
because it doesn't support subgroups. It will automatically support
them with the introduction of the multi-backend queries.
Bug: T169640
Change-Id: I3d6859b052cbafd7786c9b0b96f0abc0e965f87a
---
M README.md
M cumin/backends/__init__.py
M cumin/grammar.py
M cumin/query.py
M cumin/tests/fixtures/grammar/invalid_grammars.txt
M cumin/tests/fixtures/grammar/valid_grammars.txt
M cumin/tests/unit/test_query.py
7 files changed, 96 insertions(+), 10 deletions(-)
Approvals:
Giuseppe Lavagetto: Looks good to me, but someone else must approve
jenkins-bot: Verified
Volans: Looks good to me, approved
diff --git a/README.md b/README.md
index 6939b4e..b4edde5 100644
--- a/README.md
+++ b/README.md
@@ -39,6 +39,9 @@
The available categories are:
+- `A`: for aliases. Aliases are automatically loaded from
`{backend_name}_aliases.yaml` files, if they are present in
+ the same directory of the main configuration file. They are also
automatically replaced with their values and nesting
+ aliases is allowed.
- `F`: for querying facts
- `R`: for querying resources
@@ -64,6 +67,7 @@
- Category based key-value selection: `R:ResourceName = ResourceValue`
- A complex selection for facts:
`host10[10-42].*.domain or (not F:key1 = value1 and host10*) or (F:key2 >
value2 and F:key3 ~ '^value[0-9]+')`
+- Alias selection: `A:group1`
Backus-Naur form (BNF) of the grammar:
diff --git a/cumin/backends/__init__.py b/cumin/backends/__init__.py
index 120afb9..bdbb231 100644
--- a/cumin/backends/__init__.py
+++ b/cumin/backends/__init__.py
@@ -31,7 +31,7 @@
"""Add a category token to the query 'F:key = value'.
Arguments:
- category -- the category of the token, one of cumin.grammar.CATEGORIES
+ category -- the category of the token, one of cumin.grammar.CATEGORIES
excluding the alias one.
key -- the key for this category
value -- the value to match, if not specified the key itself will
be matched [optional, default: None]
operator -- the comparison operator to use, one of
cumin.grammar.OPERATORS [optional: default: =]
diff --git a/cumin/grammar.py b/cumin/grammar.py
index 95de3ef..d60759f 100644
--- a/cumin/grammar.py
+++ b/cumin/grammar.py
@@ -4,6 +4,7 @@
# Available categories
CATEGORIES = (
+ 'A', # Alias, it will be automatically replaced, the backends will never
see it.
'F', # Fact
'R', # Resource
)
diff --git a/cumin/query.py b/cumin/query.py
index 12fc199..a816882 100644
--- a/cumin/query.py
+++ b/cumin/query.py
@@ -6,6 +6,7 @@
from ClusterShell.NodeSet import NodeSet
from pyparsing import ParseResults
+from cumin.backends import InvalidQueryError
from cumin.grammar import grammar
@@ -45,6 +46,7 @@
self.logger = logger or logging.getLogger(__name__)
self.query_string = query_string.strip()
self.query = Query.new(config, logger=self.logger)
+ self.aliases = config.get(config['backend'], {}).get('aliases', {})
self.level = 0 # Nesting level for sub-groups
def build(self):
@@ -60,10 +62,10 @@
Arguments:
token -- a single token returned by the grammar parsing
- level -- Nesting level in case of sub-groups in the query [optional,
default: 0]
+ level -- nesting level in case of sub-groups in the query [optional,
default: 0]
"""
if not isinstance(token, ParseResults):
- raise RuntimeError("Invalid query string syntax '{query}'. Token
is '{token}'".format(
+ raise InvalidQueryError("Invalid query string syntax '{query}'.
Token is '{token}'".format(
query=self.query_string, token=token))
token_dict = token.asDict()
@@ -71,14 +73,15 @@
for subtoken in token:
self._parse_token(subtoken, level=(level + 1))
else:
- self._build_token(token_dict, level)
+ if not self._replace_alias(token_dict, level):
+ self._build_token(token_dict, level)
def _build_token(self, token_dict, level):
"""Build a token into the query object for the configured backend.
Arguments:
token_dict -- the dictionary of the parsed token returned by the
grammar parsing
- level -- Nesting level in the query
+ level -- nesting level in the query
"""
keys = token_dict.keys()
@@ -103,3 +106,32 @@
elif 'category' in keys:
self.query.add_category(**token_dict)
+
+ def _replace_alias(self, token_dict, level):
+ """Replace any alias in the query in a recursive way, alias can
reference other aliases.
+
+ Return True if a replacement was made, False otherwise. Raise
InvalidQueryError on failure.
+
+ Arguments:
+ token_dict -- the dictionary of the parsed token returned by the
grammar parsing
+ level -- nesting level in the query
+ """
+ keys = token_dict.keys()
+ if 'category' not in keys or token_dict['category'] != 'A':
+ return False
+
+ if 'operator' in keys or 'value' in keys:
+ raise InvalidQueryError('Invalid alias syntax, aliases can be only
of the form: A:alias_name')
+
+ alias_name = token_dict['key']
+ if alias_name not in self.aliases:
+ raise InvalidQueryError("Unable to find alias replacement for
'{alias}' in the configuration".format(
+ alias=alias_name))
+
+ neg = 'not ' if 'neg' in keys and token_dict['neg'] else ''
+ alias = '{neg}({alias})'.format(neg=neg,
alias=self.aliases[alias_name])
+ parsed_alias = grammar.parseString(alias, parseAll=True)
+ for token in parsed_alias:
+ self._parse_token(token, level=level)
+
+ return True
diff --git a/cumin/tests/fixtures/grammar/invalid_grammars.txt
b/cumin/tests/fixtures/grammar/invalid_grammars.txt
index c8ce883..dfc84a9 100644
--- a/cumin/tests/fixtures/grammar/invalid_grammars.txt
+++ b/cumin/tests/fixtures/grammar/invalid_grammars.txt
@@ -11,3 +11,4 @@
(F:key1 < 5 and () or hostname)
(F:key1 < 5 and (hostname not F:key2 = value))
F:key1 = value or (F:key2 < 5 and (hostname or F:key3 = (value)) and F:key4) =
value
+Z:invalid_category
diff --git a/cumin/tests/fixtures/grammar/valid_grammars.txt
b/cumin/tests/fixtures/grammar/valid_grammars.txt
index ba5b00f..edd74ce 100644
--- a/cumin/tests/fixtures/grammar/valid_grammars.txt
+++ b/cumin/tests/fixtures/grammar/valid_grammars.txt
@@ -31,3 +31,4 @@
not F:key1 = value or not ( not F:key2 < 5 and not (not hostname or not F:key3
= value)) and not F:key4 = value
not F:key1 or not ( not F:key2 < 5 and not (not hostname or not F:key3)) and
not F:key4 = value
(F:key1 = value and F:key2 = "another value") or (F:key3 = Value and F:key4 >
8.1)
+A:custom_alias
diff --git a/cumin/tests/unit/test_query.py b/cumin/tests/unit/test_query.py
index 4dbd2ee..aefe8b8 100644
--- a/cumin/tests/unit/test_query.py
+++ b/cumin/tests/unit/test_query.py
@@ -10,7 +10,7 @@
from ClusterShell.NodeSet import NodeSet
from pyparsing import ParseException
-from cumin.backends import BaseQuery
+from cumin.backends import BaseQuery, InvalidQueryError
from cumin.query import Query, QueryBuilder
@@ -58,7 +58,15 @@
query_string = 'host1 or (not F:key1 = value and R:key2 ~ regex) or host2'
invalid_query_string = 'host1 and or not F:key1 value'
- config = {'backend': 'test_backend'}
+ config = {
+ 'backend': 'test_backend',
+ 'test_backend': {
+ 'aliases': {
+ 'group1': 'host1 or host10[10-22]',
+ 'nested_group': 'host10[40-42] or A:group1',
+ },
+ },
+ }
@mock.patch('cumin.query.Query', QueryFactory)
def test_instantiation(self):
@@ -76,7 +84,7 @@
query_builder.build()
query_builder.query.add_hosts.assert_has_calls(
- [mock.call(hosts=NodeSet.fromlist(['host1'])),
mock.call(hosts=NodeSet.fromlist(['host2']))])
+ [mock.call(hosts=NodeSet('host1')),
mock.call(hosts=NodeSet('host2'))])
query_builder.query.add_or.assert_has_calls([mock.call(), mock.call()])
query_builder.query.open_subgroup.assert_called_once_with()
query_builder.query.add_category.assert_has_calls([
@@ -86,11 +94,50 @@
query_builder.query.close_subgroup.assert_called_once_with()
@mock.patch('cumin.query.Query', QueryFactory)
+ def test_build_valid_with_aliases(self):
+ """QueryBuilder.build() should replace any aliases and build the query
object for a valid query."""
+ query_builder = QueryBuilder('host100 or A:group1 or host2',
self.config)
+ query_builder.build()
+
+ query_builder.query.add_hosts.assert_has_calls(
+ [mock.call(hosts=NodeSet('host100')),
mock.call(hosts=NodeSet('host1')),
+ mock.call(hosts=NodeSet('host10[10-22]')),
mock.call(hosts=NodeSet('host2'))])
+ query_builder.query.add_or.assert_has_calls([mock.call(), mock.call(),
mock.call()])
+ query_builder.query.open_subgroup.assert_called_once_with()
+ query_builder.query.close_subgroup.assert_called_once_with()
+
+ @mock.patch('cumin.query.Query', QueryFactory)
+ def test_build_valid_with_nested_aliases(self): # pylint:
disable=invalid-name
+ """QueryBuilder.build() should replace any aliases and build the query
object for a valid query."""
+ query_builder = QueryBuilder('host100 or A:nested_group', self.config)
+ query_builder.build()
+
+ query_builder.query.add_hosts.assert_has_calls(
+ [mock.call(hosts=NodeSet('host100')),
mock.call(hosts=NodeSet('host10[40-42]')),
+ mock.call(hosts=NodeSet('host1')),
mock.call(hosts=NodeSet('host10[10-22]'))])
+ query_builder.query.add_or.assert_has_calls([mock.call(), mock.call()])
+ query_builder.query.open_subgroup.assert_has_calls([mock.call(),
mock.call()])
+
+ @mock.patch('cumin.query.Query', QueryFactory)
+ def test_build_invalid_alias_syntax(self):
+ """QueryBuilder.build() should raise InvalidQueryError if an alias has
invalid syntax."""
+ query_builder = QueryBuilder('host1 or A:name = value', self.config)
+ with pytest.raises(InvalidQueryError, match='Invalid alias syntax,
aliases can be only of the form'):
+ query_builder.build()
+
+ @mock.patch('cumin.query.Query', QueryFactory)
+ def test_build_missing_alias(self):
+ """QueryBuilder.build() should raise InvalidQueryError if a non
existent alias is found."""
+ query_builder = QueryBuilder('host1 or A:non_existent_group',
self.config)
+ with pytest.raises(InvalidQueryError, match='Unable to find alias
replacement for'):
+ query_builder.build()
+
+ @mock.patch('cumin.query.Query', QueryFactory)
def test_build_glob_host(self):
"""QueryBuilder.build() should parse a glob host."""
query_builder = QueryBuilder('host1*', self.config)
query_builder.build()
-
query_builder.query.add_hosts.assert_called_once_with(hosts=NodeSet.fromlist(['host1*']))
+
query_builder.query.add_hosts.assert_called_once_with(hosts=NodeSet('host1*'))
@mock.patch('cumin.query.Query', QueryFactory)
def test_build_invalid(self):
@@ -103,5 +150,5 @@
def test__parse_token(self):
"""QueryBuilder._parse_token() should raise RuntimeError for an
invalid token."""
query_builder = QueryBuilder(self.invalid_query_string, self.config)
- with pytest.raises(RuntimeError, match='Invalid query string syntax'):
+ with pytest.raises(InvalidQueryError, match='Invalid query string
syntax'):
query_builder._parse_token('invalid_token') # pylint:
disable=protected-access
--
To view, visit https://gerrit.wikimedia.org/r/363748
To unsubscribe, visit https://gerrit.wikimedia.org/r/settings
Gerrit-MessageType: merged
Gerrit-Change-Id: I3d6859b052cbafd7786c9b0b96f0abc0e965f87a
Gerrit-PatchSet: 1
Gerrit-Project: operations/software/cumin
Gerrit-Branch: master
Gerrit-Owner: Volans <[email protected]>
Gerrit-Reviewer: Faidon Liambotis <[email protected]>
Gerrit-Reviewer: Gehel <[email protected]>
Gerrit-Reviewer: Giuseppe Lavagetto <[email protected]>
Gerrit-Reviewer: Muehlenhoff <[email protected]>
Gerrit-Reviewer: Volans <[email protected]>
Gerrit-Reviewer: jenkins-bot <>
_______________________________________________
MediaWiki-commits mailing list
[email protected]
https://lists.wikimedia.org/mailman/listinfo/mediawiki-commits
