From 4174506f9249752f1282d721d924da92cd55f7a6 Mon Sep 17 00:00:00 2001 From: Kefu Chai Date: Mon, 18 Nov 2019 16:34:48 +0800 Subject: [PATCH] mgr/orchestrator: do not try to iterate through None `PlacementSpec` instances could be constructed with `nodes=None`, see `OrchestratorCli._mds_add()` in orchestrator_cli/module.py, so we need to handle the case of `None`. also, it's dangerous to pass a list as the default parameter to a method in python, as the value of reference point to that list will be stored. and multiple calls of that method will share the same instance of list. which is not expected under most circumstances. in this change, list comprehension is unsed instead of more functional `list(map(...))`, because, the behavior of `map()` in python3 is quite different from that in python2. in python3, `map()` returns an iterator. and `list()` actually materializes the iterator by walking through it, and `list()` changes the iterator by advancing it. so, to improve the readability and to reduce the menta load of developers, list comprehension is better in most cases. Signed-off-by: Kefu Chai --- src/pybind/mgr/orchestrator.py | 7 +++++-- 1 file changed, 5 insertions(+), 2 deletions(-) diff --git a/src/pybind/mgr/orchestrator.py b/src/pybind/mgr/orchestrator.py index ba2156e31915..ad6923bb1191 100644 --- a/src/pybind/mgr/orchestrator.py +++ b/src/pybind/mgr/orchestrator.py @@ -667,10 +667,13 @@ class PlacementSpec(object): """ For APIs that need to specify a node subset """ - def __init__(self, label=None, nodes=[]): + def __init__(self, label=None, nodes=None): self.label = label + if nodes: + self.nodes = [parse_host_specs(x, require_network=False) for x in nodes] + else: + self.nodes = [] - self.nodes = [parse_host_specs(x, require_network=False) for x in nodes] def handle_type_error(method): @wraps(method) -- 2.47.3