From 3396b9fa0e3b0f426ebba88cc33f827ba0706661 Mon Sep 17 00:00:00 2001 From: Ian Pittwood Date: Thu, 9 May 2019 10:50:05 -0500 Subject: [PATCH] Removes remaining yapf: disable statements The initial addition of yapf into Spyglass caused a few alignment issues that were temporarily fixed by disabling yapf. This change adds a knob to the yapf configuration that causes long function statements to always break before the first statement. This results in more consistent, visually pleasing code. Change-Id: I18f9a7677c61524fed12e71a2ecf1003a6ee0ad9 --- setup.cfg | 1 + spyglass/cli.py | 119 ++++++++++-------- spyglass/data_extractor/base.py | 13 +- spyglass/data_extractor/custom_exceptions.py | 5 +- .../plugins/formation/formation.py | 35 +++--- .../plugins/tugboat/check_exceptions.py | 5 +- .../plugins/tugboat/excel_parser.py | 47 +++---- .../data_extractor/plugins/tugboat/tugboat.py | 16 +-- spyglass/parser/engine.py | 29 +++-- spyglass/site_processors/site_processor.py | 12 +- 10 files changed, 155 insertions(+), 127 deletions(-) diff --git a/setup.cfg b/setup.cfg index 13ab373..528f1dc 100644 --- a/setup.cfg +++ b/setup.cfg @@ -36,3 +36,4 @@ allow_split_before_dict_value = false blank_line_before_nested_class_or_def = true blank_line_before_module_docstring = true split_before_logical_operator = false +split_before_first_argument = true diff --git a/spyglass/cli.py b/spyglass/cli.py index e5ada66..a473ec1 100644 --- a/spyglass/cli.py +++ b/spyglass/cli.py @@ -37,9 +37,10 @@ def tugboat_required_callback(ctx, param, value): if 'plugin_type' not in ctx.params or \ ctx.params['plugin_type'] == 'tugboat': if not value: - raise click.UsageError('%s is required for the tugboat ' - 'plugin.' % str(param.name), - ctx=ctx) + raise click.UsageError( + '%s is required for the tugboat ' + 'plugin.' % str(param.name), + ctx=ctx) return value @@ -48,9 +49,10 @@ def formation_required_callback(ctx, param, value): if 'plugin_type' in ctx.params: if ctx.params['plugin_type'] == 'formation': if not value: - raise click.UsageError('%s is required for the ' - 'formation plugin.' % str(param.name), - ctx=ctx) + raise click.UsageError( + '%s is required for the ' + 'formation plugin.' % str(param.name), + ctx=ctx) return value return ['', '', ''] @@ -62,7 +64,7 @@ PLUGIN_TYPE_OPTION = click.option( type=click.Choice(['formation', 'tugboat']), default='tugboat', show_default=True, - help='The plugin type to use.') # yapf: disable + help='The plugin type to use.') # TODO(ianp): Either provide a prompt for passwords or use environment # variable so passwords are no longer plain text @@ -71,8 +73,9 @@ FORMATION_TARGET_OPTION = click.option( '--formation-target', 'formation_target', nargs=3, - help=('Target URL, username, and password for formation plugin. Required ' - 'for formation plugin.'), + help=( + 'Target URL, username, and password for formation plugin. Required ' + 'for formation plugin.'), callback=formation_required_callback) INTERMEDIARY_DIR_OPTION = click.option( @@ -97,8 +100,9 @@ EXCEL_SPEC_OPTION = click.option( '--excel-spec', 'excel_spec', type=click.Path(exists=True, readable=True, dir_okay=False), - help=('Path to the Excel specification YAML file for the engineering ' - 'Excel file. Required for tugboat plugin.'), + help=( + 'Path to the Excel specification YAML file for the engineering ' + 'Excel file. Required for tugboat plugin.'), callback=tugboat_required_callback) SITE_CONFIGURATION_FILE_OPTION = click.option( @@ -135,11 +139,12 @@ MANIFEST_DIR_OPTION = click.option( @click.group(context_settings=CONTEXT_SETTINGS) -@click.option('-v', - '--verbose', - is_flag=True, - default=False, - help='Enable debug messages in log.') +@click.option( + '-v', + '--verbose', + is_flag=True, + default=False, + help='Enable debug messages in log.') def main(*, verbose): """CLI for Airship Spyglass""" if verbose: @@ -149,8 +154,9 @@ def main(*, verbose): logging.basicConfig(format=LOG_FORMAT, level=log_level) -def _intermediary_helper(plugin_type, formation_data, site, excel_file, - excel_spec, additional_configuration): +def _intermediary_helper( + plugin_type, formation_data, site, excel_file, excel_spec, + additional_configuration): LOG.info("Generating Intermediary yaml") plugin_type = plugin_type plugin_class = None @@ -170,13 +176,14 @@ def _intermediary_helper(plugin_type, formation_data, site, excel_file, # Extract data from plugin data source LOG.info("Extract data from plugin data source") data_extractor = plugin_class(site) - plugin_conf = data_extractor.get_plugin_conf({ - 'excel': excel_file, - 'excel_spec': excel_spec, - 'formation_url': formation_data[0], - 'formation_user': formation_data[1], - 'formation_password': formation_data[2] - }) # yapf: disable + plugin_conf = data_extractor.get_plugin_conf( + { + 'excel': excel_file, + 'excel_spec': excel_spec, + 'formation_url': formation_data[0], + 'formation_user': formation_data[1], + 'formation_password': formation_data[2] + }) data_extractor.set_config_opts(plugin_conf) data_extractor.extract_data() @@ -186,8 +193,9 @@ def _intermediary_helper(plugin_type, formation_data, site, excel_file, with open(additional_config, 'r') as config: raw_data = config.read() additional_config_data = yaml.safe_load(raw_data) - LOG.debug("Additional config data:\n{}".format( - pprint.pformat(additional_config_data))) + LOG.debug( + "Additional config data:\n{}".format( + pprint.pformat(additional_config_data))) LOG.info( "Apply additional configuration from:{}".format(additional_config)) @@ -202,9 +210,10 @@ def _intermediary_helper(plugin_type, formation_data, site, excel_file, return process_input_ob -@main.command('i', - short_help='generate intermediary', - help='Generates an intermediary file from passed excel data.') +@main.command( + 'i', + short_help='generate intermediary', + help='Generates an intermediary file from passed excel data.') @PLUGIN_TYPE_OPTION @FORMATION_TARGET_OPTION @INTERMEDIARY_DIR_OPTION @@ -212,20 +221,21 @@ def _intermediary_helper(plugin_type, formation_data, site, excel_file, @EXCEL_SPEC_OPTION @SITE_CONFIGURATION_FILE_OPTION @SITE_NAME_CONFIGURATION_OPTION -def generate_intermediary(*, plugin_type, formation_target, intermediary_dir, - excel_file, excel_spec, site_configuration, - site_name): - process_input_ob = _intermediary_helper(plugin_type, formation_target, - site_name, excel_file, excel_spec, - site_configuration) +def generate_intermediary( + *, plugin_type, formation_target, intermediary_dir, excel_file, + excel_spec, site_configuration, site_name): + process_input_ob = _intermediary_helper( + plugin_type, formation_target, site_name, excel_file, excel_spec, + site_configuration) LOG.info("Generate intermediary yaml") process_input_ob.generate_intermediary_yaml() process_input_ob.dump_intermediary_file(intermediary_dir) -@main.command('m', - short_help='generates manifest and intermediary', - help='Generates manifest and intermediary files.') +@main.command( + 'm', + short_help='generates manifest and intermediary', + help='Generates manifest and intermediary files.') @click.option( '-i', '--save-intermediary', @@ -243,14 +253,13 @@ def generate_intermediary(*, plugin_type, formation_target, intermediary_dir, @SITE_NAME_CONFIGURATION_OPTION @TEMPLATE_DIR_OPTION @MANIFEST_DIR_OPTION -def generate_manifests_and_intermediary(*, save_intermediary, plugin_type, - formation_target, intermediary_dir, - excel_file, excel_spec, - site_configuration, site_name, - template_dir, manifest_dir): - process_input_ob = _intermediary_helper(plugin_type, formation_target, - site_name, excel_file, excel_spec, - site_configuration) +def generate_manifests_and_intermediary( + *, save_intermediary, plugin_type, formation_target, intermediary_dir, + excel_file, excel_spec, site_configuration, site_name, template_dir, + manifest_dir): + process_input_ob = _intermediary_helper( + plugin_type, formation_target, site_name, excel_file, excel_spec, + site_configuration) LOG.info("Generate intermediary yaml") intermediary_yaml = process_input_ob.generate_intermediary_yaml() if save_intermediary: @@ -264,15 +273,17 @@ def generate_manifests_and_intermediary(*, save_intermediary, plugin_type, processor_engine.render_template(template_dir) -@main.command('mi', - short_help='generates manifest from intermediary', - help='Generate manifest files from specified intermediary file.') -@click.argument('intermediary_file', - type=click.Path(exists=True, readable=True, dir_okay=False)) +@main.command( + 'mi', + short_help='generates manifest from intermediary', + help='Generate manifest files from specified intermediary file.') +@click.argument( + 'intermediary_file', + type=click.Path(exists=True, readable=True, dir_okay=False)) @TEMPLATE_DIR_OPTION @MANIFEST_DIR_OPTION -def generate_manifests_using_intermediary(*, intermediary_file, template_dir, - manifest_dir): +def generate_manifests_using_intermediary( + *, intermediary_file, template_dir, manifest_dir): LOG.info("Loading intermediary from user provided input") with open(intermediary_file, 'r') as f: raw_data = f.read() diff --git a/spyglass/data_extractor/base.py b/spyglass/data_extractor/base.py index a4c740c..35ffa08 100755 --- a/spyglass/data_extractor/base.py +++ b/spyglass/data_extractor/base.py @@ -304,8 +304,8 @@ class BaseDataSourcePlugin(metaclass=abc.ABCMeta): baremetal[rack_name][host_name] = temp_host - LOG.debug("Baremetal information:\n{}".format( - pprint.pformat(baremetal))) + LOG.debug( + "Baremetal information:\n{}".format(pprint.pformat(baremetal))) return baremetal @@ -349,8 +349,9 @@ class BaseDataSourcePlugin(metaclass=abc.ABCMeta): domain_data = self.get_domain_name(self.region) site_info["domain"] = domain_data - LOG.debug("Extracted site information:\n{}".format( - pprint.pformat(site_info))) + LOG.debug( + "Extracted site information:\n{}".format( + pprint.pformat(site_info))) return site_info @@ -401,8 +402,8 @@ class BaseDataSourcePlugin(metaclass=abc.ABCMeta): network_data["vlan_network_data"][net["name"]] = tmp_net - LOG.debug("Extracted network data:\n{}".format( - pprint.pformat(network_data))) + LOG.debug( + "Extracted network data:\n{}".format(pprint.pformat(network_data))) return network_data def extract_data(self): diff --git a/spyglass/data_extractor/custom_exceptions.py b/spyglass/data_extractor/custom_exceptions.py index c98456b..bc8fc07 100644 --- a/spyglass/data_extractor/custom_exceptions.py +++ b/spyglass/data_extractor/custom_exceptions.py @@ -35,8 +35,9 @@ class NoSpecMatched(BaseError): def display_error(self): # FIXME (Ian Pittwood): use log instead of print - print("No spec matched. Following are the available specs:\n".format( - self.specs)) + print( + "No spec matched. Following are the available specs:\n".format( + self.specs)) sys.exit(1) diff --git a/spyglass/data_extractor/plugins/formation/formation.py b/spyglass/data_extractor/plugins/formation/formation.py index d905e32..1f80379 100755 --- a/spyglass/data_extractor/plugins/formation/formation.py +++ b/spyglass/data_extractor/plugins/formation/formation.py @@ -308,27 +308,29 @@ class FormationPlugin(BaseDataSourcePlugin): zone_id = self._get_zone_id_by_name(zone) device_api = formation_client.DevicesApi(self.formation_api_client) control_hosts = device_api.zones_zone_id_control_nodes_get(zone_id) - compute_hosts = device_api.zones_zone_id_devices_get(zone_id, - type="KVM") + compute_hosts = device_api.zones_zone_id_devices_get( + zone_id, type="KVM") hosts_list = [] for host in control_hosts: self.device_name_id_mapping[host.aic_standard_name] = host.id - hosts_list.append({ - "name": host.aic_standard_name, - "type": "controller", - "rack_name": host.rack_name, - "host_profile": host.host_profile_name, - }) + hosts_list.append( + { + "name": host.aic_standard_name, + "type": "controller", + "rack_name": host.rack_name, + "host_profile": host.host_profile_name, + }) for host in compute_hosts: self.device_name_id_mapping[host.aic_standard_name] = host.id - hosts_list.append({ - "name": host.aic_standard_name, - "type": "compute", - "rack_name": host.rack_name, - "host_profile": host.host_profile_name, - }) + hosts_list.append( + { + "name": host.aic_standard_name, + "type": "compute", + "rack_name": host.rack_name, + "host_profile": host.host_profile_name, + }) """ for host in itertools.chain(control_hosts, compute_hosts): self.device_name_id_mapping[host.aic_standard_name] = host.id @@ -406,8 +408,9 @@ class FormationPlugin(BaseDataSourcePlugin): name = self._get_network_name_from_vlan_name( vlan_.vlan.name) ipv4 = vlan_.vlan.ipv4[0].ip - LOG.debug("vlan:{},name:{},ip:{},vlan_name:{}".format( - vlan_.vlan.vlan_id, name, ipv4, vlan_.vlan.name)) + LOG.debug( + "vlan:{},name:{},ip:{},vlan_name:{}".format( + vlan_.vlan.vlan_id, name, ipv4, vlan_.vlan.name)) # TODD(pg710r) This code needs to extended to support ipv4 # and ipv6 # ip_[host][name] = {'ipv4': ipv4} diff --git a/spyglass/data_extractor/plugins/tugboat/check_exceptions.py b/spyglass/data_extractor/plugins/tugboat/check_exceptions.py index 94bc0f2..49ac3b4 100644 --- a/spyglass/data_extractor/plugins/tugboat/check_exceptions.py +++ b/spyglass/data_extractor/plugins/tugboat/check_exceptions.py @@ -33,5 +33,6 @@ class NoSpecMatched(BaseError): self.specs = excel_specs def display_error(self): - print("No spec matched. Following are the available specs:\n".format( - self.specs)) + print( + "No spec matched. Following are the available specs:\n".format( + self.specs)) diff --git a/spyglass/data_extractor/plugins/tugboat/excel_parser.py b/spyglass/data_extractor/plugins/tugboat/excel_parser.py index 41bebd6..3f4d1c7 100755 --- a/spyglass/data_extractor/plugins/tugboat/excel_parser.py +++ b/spyglass/data_extractor/plugins/tugboat/excel_parser.py @@ -109,10 +109,10 @@ class ExcelParser(object): host_profile = ws.cell(row=row, column=host_profile_col).value try: if host_profile is None: - raise RuntimeError("No value read from " - "{} sheet:{} row:{}, col:{}".format( - self.file_name, self.spec, row, - host_profile_col)) + raise RuntimeError( + "No value read from " + "{} sheet:{} row:{}, col:{}".format( + self.file_name, self.spec, row, host_profile_col)) except RuntimeError as rerror: LOG.critical(rerror) sys.exit("Tugboat exited!!") @@ -123,10 +123,12 @@ class ExcelParser(object): "type": type, # FIXME (Ian Pittwood): shadows type built-in } row += 1 - LOG.debug("ipmi data extracted from excel:\n{}".format( - pprint.pformat(ipmi_data))) - LOG.debug("host data extracted from excel:\n{}".format( - pprint.pformat(hosts))) + LOG.debug( + "ipmi data extracted from excel:\n{}".format( + pprint.pformat(ipmi_data))) + LOG.debug( + "host data extracted from excel:\n{}".format( + pprint.pformat(hosts))) return [ipmi_data, hosts] def get_private_vlan_data(self, ws): @@ -145,8 +147,8 @@ class ExcelParser(object): vlan = vlan.lower() vlan_data[vlan] = cell_value row += 1 - LOG.debug("vlan data extracted from excel:\n%s" % - pprint.pformat(vlan_data)) + LOG.debug( + "vlan data extracted from excel:\n%s" % pprint.pformat(vlan_data)) return vlan_data def get_private_network_data(self): @@ -231,8 +233,9 @@ class ExcelParser(object): if cell_value: network_data["oob"]["subnet"].append(self.sanitize(cell_value)) col += 1 - LOG.debug("public network data extracted from excel:\n%s" % - pprint.pformat(network_data)) + LOG.debug( + "public network data extracted from excel:\n%s" % + pprint.pformat(network_data)) return network_data def get_site_info(self): @@ -260,15 +263,15 @@ class ExcelParser(object): ntp_servers = ws.cell(row=ntp_row, column=ntp_col).value try: if dns_servers is None: - raise RuntimeError("No value for dns_server from: " - "{} Sheet:'{}' Row:{} Col:{}".format( - self.file_name, provided_sheetname, - dns_row, dns_col)) + raise RuntimeError( + "No value for dns_server from: " + "{} Sheet:'{}' Row:{} Col:{}".format( + self.file_name, provided_sheetname, dns_row, dns_col)) if ntp_servers is None: - raise RuntimeError("No value for ntp_server from: " - "{} Sheet:'{}' Row:{} Col:{}".format( - self.file_name, provided_sheetname, - ntp_row, ntp_col)) + raise RuntimeError( + "No value for ntp_server from: " + "{} Sheet:'{}' Row:{} Col:{}".format( + self.file_name, provided_sheetname, ntp_row, ntp_col)) except RuntimeError as rerror: LOG.critical(rerror) sys.exit("Tugboat exited!!") @@ -380,8 +383,8 @@ class ExcelParser(object): }, "site_info": site_info_data, } - LOG.debug("Location data extracted from excel:\n%s" % - pprint.pformat(data)) + LOG.debug( + "Location data extracted from excel:\n%s" % pprint.pformat(data)) return data def combine_excel_design_specs(self, filenames): diff --git a/spyglass/data_extractor/plugins/tugboat/tugboat.py b/spyglass/data_extractor/plugins/tugboat/tugboat.py index 15b047f..a9e231c 100755 --- a/spyglass/data_extractor/plugins/tugboat/tugboat.py +++ b/spyglass/data_extractor/plugins/tugboat/tugboat.py @@ -108,11 +108,12 @@ class TugboatPlugin(BaseDataSourcePlugin): host_list = [] for rack in rackwise_hosts.keys(): for host in rackwise_hosts[rack]: - host_list.append({ - "rack_name": rack, - "name": host, - "host_profile": ipmi_data[host]["host_profile"], - }) + host_list.append( + { + "rack_name": rack, + "name": host, + "host_profile": ipmi_data[host]["host_profile"], + }) return host_list def get_networks(self, region): @@ -152,8 +153,9 @@ class TugboatPlugin(BaseDataSourcePlugin): tmp_vlan["name"] = "ingress" tmp_vlan["subnet"] = net_val vlan_list.append(tmp_vlan) - LOG.debug("vlan list extracted from tugboat:\n{}".format( - pprint.pformat(vlan_list))) + LOG.debug( + "vlan list extracted from tugboat:\n{}".format( + pprint.pformat(vlan_list))) return vlan_list def get_ips(self, region, host=None): diff --git a/spyglass/parser/engine.py b/spyglass/parser/engine.py index 0d593c2..467d2a9 100755 --- a/spyglass/parser/engine.py +++ b/spyglass/parser/engine.py @@ -72,8 +72,8 @@ class ProcessDataSource(object): ["subnet"] [0]) - LOG.debug("Network subnets:\n{}".format( - pprint.pformat(network_subnets))) + LOG.debug( + "Network subnets:\n{}".format(pprint.pformat(network_subnets))) return network_subnets def _get_genesis_node_details(self): @@ -84,8 +84,9 @@ class ProcessDataSource(object): if rack_hosts[host]["type"] == "genesis": self.genesis_node = rack_hosts[host] self.genesis_node["name"] = host - LOG.debug("Genesis Node Details:\n{}".format( - pprint.pformat(self.genesis_node))) + LOG.debug( + "Genesis Node Details:\n{}".format( + pprint.pformat(self.genesis_node))) def _validate_intermediary_data(self, data): """Validates the intermediary data before generating manifests. @@ -226,8 +227,9 @@ class ProcessDataSource(object): host_networks[net] = str(ips[host_idx + default_ip_offset]) host_idx = host_idx + 1 - LOG.debug("Updated baremetal host:\n{}".format( - pprint.pformat(self.data["baremetal"]))) + LOG.debug( + "Updated baremetal host:\n{}".format( + pprint.pformat(self.data["baremetal"]))) def _update_vlan_net_data(self, rule_data): """Offset allocation rules to determine ip address range(s) @@ -259,8 +261,9 @@ class ProcessDataSource(object): (vlan_network_data_["ingress"] ["subnet"] [0]) - LOG.debug("Updated network bgp data:\n{}".format( - pprint.pformat(self.data["network"]["bgp"]))) + LOG.debug( + "Updated network bgp data:\n{}".format( + pprint.pformat(self.data["network"]["bgp"]))) LOG.info("Apply network design rules:vlan") # Apply rules to vlan networks @@ -306,8 +309,9 @@ class ProcessDataSource(object): routes = [] vlan_network_data_[net_type]["routes"] = routes - LOG.debug("Updated vlan network data:\n{}".format( - pprint.pformat(vlan_network_data_))) + LOG.debug( + "Updated vlan network data:\n{}".format( + pprint.pformat(vlan_network_data_))) def load_extracted_data_from_data_source(self, extracted_data): """Function called from cli.py to pass extracted data @@ -324,8 +328,9 @@ class ProcessDataSource(object): LOG.info("Loading plugin data source") self.data = extracted_data - LOG.debug("Extracted data from plugin:\n{}".format( - pprint.pformat(extracted_data))) + LOG.debug( + "Extracted data from plugin:\n{}".format( + pprint.pformat(extracted_data))) # Uncomment following segment for debugging purpose. # extracted_file = "extracted_file.yaml" # yaml_file = yaml.dump(extracted_data, default_flow_style=False) diff --git a/spyglass/site_processors/site_processor.py b/spyglass/site_processors/site_processor.py index 18c6b17..421bb9e 100644 --- a/spyglass/site_processors/site_processor.py +++ b/spyglass/site_processors/site_processor.py @@ -51,17 +51,17 @@ class SiteProcessor(BaseProcessor): for dirpath, dirs, files in os.walk(template_dir_abspath): for filename in files: - j2_env = Environment(autoescape=True, - loader=FileSystemLoader(dirpath), - trim_blocks=True) + j2_env = Environment( + autoescape=True, + loader=FileSystemLoader(dirpath), + trim_blocks=True) j2_env.filters["get_role_wise_nodes"] = \ self.get_role_wise_nodes templatefile = os.path.join(dirpath, filename) outdirs = dirpath.split("templates")[1] - outfile_path = "{}{}{}".format(site_manifest_dir, - self.yaml_data["region_name"], - outdirs) + outfile_path = "{}{}{}".format( + site_manifest_dir, self.yaml_data["region_name"], outdirs) outfile_yaml = templatefile.split(".j2")[0].split("/")[-1] outfile = outfile_path + "/" + outfile_yaml outfile_dir = os.path.dirname(outfile)