On 16 May 2013 10:23, Thomas Thrainer <[email protected]> wrote: > All functions/classes which are used outside of their defining module > (with tests as an exception) no longer have a leading underscore.
Please mention "cmdlib" in the title. > > Signed-off-by: Thomas Thrainer <[email protected]> > --- > lib/cmdlib/backup.py | 46 +++--- > lib/cmdlib/base.py | 6 +- > lib/cmdlib/cluster.py | 65 ++++---- > lib/cmdlib/common.py | 88 +++++------ > lib/cmdlib/group.py | 54 +++---- > lib/cmdlib/instance.py | 309 > ++++++++++++++++++++------------------ > lib/cmdlib/instance_migration.py | 54 +++---- > lib/cmdlib/instance_operation.py | 76 +++++----- > lib/cmdlib/instance_query.py | 32 ++-- > lib/cmdlib/instance_storage.py | 186 +++++++++++------------ > lib/cmdlib/instance_utils.py | 56 +++---- > lib/cmdlib/misc.py | 16 +- > lib/cmdlib/network.py | 14 +- > lib/cmdlib/node.py | 100 ++++++------ > lib/cmdlib/operating_system.py | 6 +- > lib/cmdlib/query.py | 32 ++-- > lib/cmdlib/tags.py | 9 +- > lib/cmdlib/test.py | 10 +- > test/py/ganeti.cmdlib_unittest.py | 132 ++++++++-------- > 19 files changed, 656 insertions(+), 635 deletions(-) > diff --git a/lib/cmdlib/backup.py b/lib/cmdlib/backup.py > @@ -102,7 +102,7 @@ class LUBackupQuery(NoHooksLU): > REQ_BGL = False > > def CheckArguments(self): > - self.expq = _ExportQuery(qlang.MakeSimpleFilter("node", self.op.nodes), > + self.expq = ExportQuery(qlang.MakeSimpleFilter("node", self.op.nodes), > ["node", "export"], self.op.use_locking) Please realign the continuation line. > @@ -470,7 +470,7 @@ class LUBackupExport(LogicalUnit): > # Remove instance if requested > if self.op.remove_instance: > feedback_fn("Removing instance %s" % instance.name) > - _RemoveInstance(self, feedback_fn, instance, > + RemoveInstance(self, feedback_fn, instance, > self.op.ignore_remove_failures) Please realign the continuation line. > diff --git a/lib/cmdlib/base.py b/lib/cmdlib/base.py > index 142981f..ee76d5c 100644 > --- a/lib/cmdlib/base.py > +++ b/lib/cmdlib/base.py > @@ -319,7 +319,7 @@ class LogicalUnit(object): > else: > assert locking.LEVEL_INSTANCE not in self.needed_locks, \ > "_ExpandAndLockInstance called with instance-level locks set" > - self.op.instance_name = _ExpandInstanceName(self.cfg, > + self.op.instance_name = ExpandInstanceName(self.cfg, > self.op.instance_name) Please realign the continuation line. > diff --git a/lib/cmdlib/cluster.py b/lib/cmdlib/cluster.py > index 3cf48b0..a00dce2 100644 > --- a/lib/cmdlib/cluster.py > +++ b/lib/cmdlib/cluster.py > @@ -760,13 +759,13 @@ class LUClusterSetParams(LogicalUnit): > constants.NDC_DEFAULTS[constants.ND_OOB_PROGRAM] > > if self.op.hv_state: > - new_hv_state = _MergeAndVerifyHvState(self.op.hv_state, > + new_hv_state = MergeAndVerifyHvState(self.op.hv_state, > self.cluster.hv_state_static) Please realign the continuation line. > self.new_hv_state = dict((hv, cluster.SimpleFillHvState(values)) > for hv, values in new_hv_state.items()) > > if self.op.disk_state: > - new_disk_state = _MergeAndVerifyDiskState(self.op.disk_state, > + new_disk_state = MergeAndVerifyDiskState(self.op.disk_state, > > self.cluster.disk_state_static) Please realign the continuation line. > @@ -774,7 +773,7 @@ class LUClusterSetParams(LogicalUnit): > for storage, svalues in new_disk_state.items()) > > if self.op.ipolicy: > - self.new_ipolicy = _GetUpdatedIPolicy(cluster.ipolicy, self.op.ipolicy, > + self.new_ipolicy = GetUpdatedIPolicy(cluster.ipolicy, self.op.ipolicy, > group_policy=False) Please realign the continuation line. > @@ -785,7 +784,7 @@ class LUClusterSetParams(LogicalUnit): > for node in inst.all_nodes)]) > new_ipolicy = objects.FillIPolicy(self.new_ipolicy, group.ipolicy) > ipol = masterd.instance.CalculateGroupIPolicy(cluster, group) > - new = _ComputeNewInstanceViolations(ipol, > + new = ComputeNewInstanceViolations(ipol, > new_ipolicy, instances, self.cfg) Please realign the continuation line. > @@ -864,7 +863,7 @@ class LUClusterSetParams(LogicalUnit): > if os_name not in self.new_osp: > self.new_osp[os_name] = {} > > - self.new_osp[os_name] = _GetUpdatedParams(self.new_osp[os_name], osp, > + self.new_osp[os_name] = GetUpdatedParams(self.new_osp[os_name], osp, > use_none=True) Please realign the continuation line. > @@ -872,7 +871,7 @@ class LUClusterSetParams(LogicalUnit): > del self.new_osp[os_name] > else: > # check the parameter validity (remote check) > - _CheckOSParams(self, False, [self.cfg.GetMasterNode()], > + CheckOSParams(self, False, [self.cfg.GetMasterNode()], > os_name, self.new_osp[os_name]) Please realign the continuation line. > diff --git a/lib/cmdlib/common.py b/lib/cmdlib/common.py > index 4571836..a314e7a 100644 > --- a/lib/cmdlib/common.py > +++ b/lib/cmdlib/common.py > @@ -414,7 +414,7 @@ def _ComputeNewInstanceViolations(old_ipolicy, > new_ipolicy, instances, cfg): > _ComputeViolatingInstances(old_ipolicy, instances, cfg)) > > > -def _GetUpdatedParams(old_params, update_dict, > +def GetUpdatedParams(old_params, update_dict, > use_default=True, use_none=False): Please realign the continuation line. > @@ -577,7 +577,7 @@ def _ComputeMinMaxSpec(name, qualifier, ispecs, value): > return None > > > -def _ComputeIPolicySpecViolation(ipolicy, mem_size, cpu_count, disk_count, > +def ComputeIPolicySpecViolation(ipolicy, mem_size, cpu_count, disk_count, > nic_count, disk_sizes, spindle_use, > disk_template, > _compute_fn=_ComputeMinMaxSpec): Please realign the continuation line. > @@ -632,8 +632,8 @@ def _ComputeIPolicySpecViolation(ipolicy, mem_size, > cpu_count, disk_count, > return ret + min_errs > > > -def _ComputeIPolicyInstanceViolation(ipolicy, instance, cfg, > - > compute_fn=_ComputeIPolicySpecViolation): > +def ComputeIPolicyInstanceViolation(ipolicy, instance, cfg, > + compute_fn=ComputeIPolicySpecViolation): Please realign the continuation line. > @@ -760,7 +760,7 @@ def _GetDefaultIAllocator(cfg, ialloc): > return ialloc > > > -def _CheckInstancesNodeGroups(cfg, instances, owned_groups, owned_nodes, > +def CheckInstancesNodeGroups(cfg, instances, owned_groups, owned_nodes, > cur_group_uuid): Please realign the continuation line. > @@ -780,13 +780,13 @@ def _CheckInstancesNodeGroups(cfg, instances, > owned_groups, owned_nodes, > assert owned_nodes.issuperset(inst.all_nodes), \ > "Instance %s's nodes changed while we kept the lock" % name > > - inst_groups = _CheckInstanceNodeGroups(cfg, name, owned_groups) > + inst_groups = CheckInstanceNodeGroups(cfg, name, owned_groups) > > assert cur_group_uuid is None or cur_group_uuid in inst_groups, \ > "Instance %s has no node in group %s" % (name, cur_group_uuid) > > > -def _CheckInstanceNodeGroups(cfg, instance_name, owned_groups, > +def CheckInstanceNodeGroups(cfg, instance_name, owned_groups, > primary_only=False): Please realign the continuation line. > diff --git a/lib/cmdlib/group.py b/lib/cmdlib/group.py > index dd51c48..29d6791 100644 > --- a/lib/cmdlib/group.py > +++ b/lib/cmdlib/group.py > @@ -363,7 +363,7 @@ class LUGroupQuery(NoHooksLU): > REQ_BGL = False > > def CheckArguments(self): > - self.gq = _GroupQuery(qlang.MakeSimpleFilter("name", self.op.names), > + self.gq = GroupQuery(qlang.MakeSimpleFilter("name", self.op.names), > self.op.output_fields, False) Please realign the continuation line. > @@ -467,16 +467,16 @@ class LUGroupSetParams(LogicalUnit): > errors.ECODE_INVAL) > > if self.op.hv_state: > - self.new_hv_state = _MergeAndVerifyHvState(self.op.hv_state, > + self.new_hv_state = MergeAndVerifyHvState(self.op.hv_state, > self.group.hv_state_static) Please realign the continuation line. > > if self.op.disk_state: > self.new_disk_state = \ > - _MergeAndVerifyDiskState(self.op.disk_state, > + MergeAndVerifyDiskState(self.op.disk_state, > self.group.disk_state_static) > Please realign the continuation line. > if self.op.ipolicy: > - self.new_ipolicy = _GetUpdatedIPolicy(self.group.ipolicy, > + self.new_ipolicy = GetUpdatedIPolicy(self.group.ipolicy, > self.op.ipolicy, > group_policy=True) Please realign the continuation line. > > @@ -485,7 +485,7 @@ class LUGroupSetParams(LogicalUnit): > instances = self.cfg.GetInstancesInfoByFilter(inst_filter).values() > gmi = ganeti.masterd.instance > violations = \ > - _ComputeNewInstanceViolations(gmi.CalculateGroupIPolicy(cluster, > + ComputeNewInstanceViolations(gmi.CalculateGroupIPolicy(cluster, > > self.group), > new_ipolicy, instances, self.cfg) Please realign the continuation line. > @@ -887,13 +887,13 @@ class LUGroupVerifyDisks(NoHooksLU): > assert self.group_uuid in owned_groups > > # Check if locked instances are still correct > - _CheckNodeGroupInstances(self.cfg, self.group_uuid, owned_instances) > + CheckNodeGroupInstances(self.cfg, self.group_uuid, owned_instances) > > # Get instance information > self.instances = dict(self.cfg.GetMultiInstanceInfo(owned_instances)) > > # Check if node groups for locked instances are still correct > - _CheckInstancesNodeGroups(self.cfg, self.instances, > + CheckInstancesNodeGroups(self.cfg, self.instances, > owned_groups, owned_nodes, self.group_uuid) Please realign the continuation line. Ok, this doesn't scale. I've gone through the whole patch and there are many more problems like this. As this series should be committed soon, I'd suggest to forget about these misalignments (also the ones above), commit the series, and then work on a solution that can be scaled. I've opened issue 468 for this. Comments? The rest of the patch, but with the interdiff that you've already sent, LGTM, thanks. Bernardo
