From 5a7d98c6b81618db5f4e76a813e565550923bd62 Mon Sep 17 00:00:00 2001 From: Jarrod Johnson Date: Thu, 21 Mar 2024 16:09:37 -0400 Subject: [PATCH] Enhance error reporting For one, when using confluent expressions, induce {} to be an error to trigger an error for someone trying to xargs something. Another is to add warnings when clear does something deliberately, but is something that might surprise a user, steering them toward what they possibly might want to do instead. --- confluent_client/bin/nodeattrib | 3 ++- confluent_client/confluent/client.py | 3 +++ .../confluent/config/configmanager.py | 20 ++++++++++++++++--- .../plugins/configuration/attributes.py | 18 ++++++++++------- 4 files changed, 33 insertions(+), 11 deletions(-) diff --git a/confluent_client/bin/nodeattrib b/confluent_client/bin/nodeattrib index f4b0331f..265fe917 100755 --- a/confluent_client/bin/nodeattrib +++ b/confluent_client/bin/nodeattrib @@ -126,13 +126,14 @@ elif options.set: argset = argset.strip() if argset: arglist += shlex.split(argset) - argset = argfile.readline() + argset = argfile.readline() session.stop_if_noderange_over(noderange, options.maxnodes) exitcode=client.updateattrib(session,arglist,nodetype, noderange, options, None) if exitcode != 0: sys.exit(exitcode) # Lists all attributes + if len(args) > 0: # setting output to all so it can search since if we do have something to search, we want to show all outputs even if it is blank. if requestargs is None: diff --git a/confluent_client/confluent/client.py b/confluent_client/confluent/client.py index ad29ff02..a7c13cd3 100644 --- a/confluent_client/confluent/client.py +++ b/confluent_client/confluent/client.py @@ -668,6 +668,9 @@ def updateattrib(session, updateargs, nodetype, noderange, options, dictassign=N for attrib in updateargs[1:]: keydata[attrib] = None for res in session.update(targpath, keydata): + for node in res.get('databynode', {}): + for warnmsg in res['databynode'][node].get('_warnings', []): + sys.stderr.write('Warning: ' + warnmsg + '\n') if 'error' in res: if 'errorcode' in res: exitcode = res['errorcode'] diff --git a/confluent_server/confluent/config/configmanager.py b/confluent_server/confluent/config/configmanager.py index 9419e7fe..dce692fd 100644 --- a/confluent_server/confluent/config/configmanager.py +++ b/confluent_server/confluent/config/configmanager.py @@ -1089,6 +1089,11 @@ class _ExpressionFormat(string.Formatter): self._nodename = nodename self._numbers = None + def _vformat(self, format_string, args, kwargs, used_args, recursion_depth, + auto_arg_index=False): + super()._vformat(format_string, args, kwargs, used_args, + recursion_depth, auto_arg_index) + def get_field(self, field_name, args, kwargs): return field_name, field_name @@ -2197,16 +2202,16 @@ class ConfigManager(object): self._notif_attribwatchers(changeset) self._bg_sync_to_file() - def clear_node_attributes(self, nodes, attributes): + def clear_node_attributes(self, nodes, attributes, warnings=None): if cfgleader: return exec_on_leader('_rpc_master_clear_node_attributes', self.tenant, nodes, attributes) if cfgstreams: exec_on_followers('_rpc_clear_node_attributes', self.tenant, nodes, attributes) - self._true_clear_node_attributes(nodes, attributes) + self._true_clear_node_attributes(nodes, attributes, warnings) - def _true_clear_node_attributes(self, nodes, attributes): + def _true_clear_node_attributes(self, nodes, attributes, warnings): # accumulate all changes into a changeset and push in one go changeset = {} realattributes = [] @@ -2229,8 +2234,17 @@ class ConfigManager(object): # delete it and check for inheritence to backfil data del nodek[attrib] self._do_inheritance(nodek, attrib, node, changeset) + if not warnings is None: + if attrib in nodek: + warnings.append('The attribute "{}" was defined specifically for the node and clearing now has a value inherited from the group "{}"'.format(attrib, nodek[attrib]['inheritedfrom'])) _addchange(changeset, node, attrib) _mark_dirtykey('nodes', node, self.tenant) + elif attrib in nodek: + if not warnings is None: + warnings.append('The attribute "{0}" is inherited from group "{1}", leaving the inherited value alone (use "{0}=" with no value to explicitly blank the value if desired)'.format(attrib, nodek[attrib]['inheritedfrom'])) + else: + if not warnings is None: + warnings.append('Attribute "{}" is either already cleared, or does not match a defined attribute (if referencing an attribute group, try a wildcard)'.format(attrib)) if ('_expressionkeys' in nodek and attrib in nodek['_expressionkeys']): recalcexpressions = True diff --git a/confluent_server/confluent/plugins/configuration/attributes.py b/confluent_server/confluent/plugins/configuration/attributes.py index c2ea83d9..a56a1aee 100644 --- a/confluent_server/confluent/plugins/configuration/attributes.py +++ b/confluent_server/confluent/plugins/configuration/attributes.py @@ -21,16 +21,16 @@ import confluent.util as util from fnmatch import fnmatch -def retrieve(nodes, element, configmanager, inputdata): +def retrieve(nodes, element, configmanager, inputdata, clearwarnbynode=None): configmanager.check_quorum() if nodes is not None: - return retrieve_nodes(nodes, element, configmanager, inputdata) + return retrieve_nodes(nodes, element, configmanager, inputdata, clearwarnbynode) elif element[0] == 'nodegroups': return retrieve_nodegroup( - element[1], element[3], configmanager, inputdata) + element[1], element[3], configmanager, inputdata, clearwarnbynode) -def retrieve_nodegroup(nodegroup, element, configmanager, inputdata): +def retrieve_nodegroup(nodegroup, element, configmanager, inputdata, clearwarnbynode): try: grpcfg = configmanager.get_nodegroup_attributes(nodegroup) except KeyError: @@ -106,10 +106,12 @@ def retrieve_nodegroup(nodegroup, element, configmanager, inputdata): raise Exception("BUGGY ATTRIBUTE FOR NODEGROUP") -def retrieve_nodes(nodes, element, configmanager, inputdata): +def retrieve_nodes(nodes, element, configmanager, inputdata, clearwarnbynode): attributes = configmanager.get_node_attributes(nodes) if element[-1] == 'all': for node in util.natural_sort(nodes): + if clearwarnbynode and node in clearwarnbynode: + yield msg.Attributes(node, {'_warnings': clearwarnbynode[node]}) theattrs = set(allattributes.node).union(set(attributes[node])) for attribute in sorted(theattrs): if attribute in attributes[node]: # have a setting for it @@ -266,6 +268,7 @@ def update_nodes(nodes, element, configmanager, inputdata): namemap[node] = rename['rename'] configmanager.rename_nodes(namemap) return yield_rename_resources(namemap, isnode=True) + clearwarnbynode = {} for node in nodes: updatenode = inputdata.get_attributes(node, allattributes.node) clearattribs = [] @@ -299,10 +302,11 @@ def update_nodes(nodes, element, configmanager, inputdata): markup = (e.text[:e.offset-1] + '-->' + e.text[e.offset-1] + '<--' + e.text[e.offset:]).strip() raise exc.InvalidArgumentException('Syntax error in attribute name: "{0}"'.format(markup)) if len(clearattribs) > 0: - configmanager.clear_node_attributes([node], clearattribs) + clearwarnbynode[node] = [] + configmanager.clear_node_attributes([node], clearattribs, warnings=clearwarnbynode[node]) updatedict[node] = updatenode try: configmanager.set_node_attributes(updatedict) except ValueError as e: raise exc.InvalidArgumentException(str(e)) - return retrieve(nodes, element, configmanager, inputdata) + return retrieve(nodes, element, configmanager, inputdata, clearwarnbynode)