From 2018277d9c99a7b958b26163ee319ceefa7e9d3c Mon Sep 17 00:00:00 2001 From: garciadeblas Date: Mon, 4 Jul 2022 08:51:14 +0200 Subject: [PATCH] Enable black and pylint in tox, and update code accordingly Change-Id: I6d2f4c7e3ac63fe9b981e3c3b61203f0efa4722a Signed-off-by: garciadeblas --- README.md | 10 ++ osmclient/client.py | 1 + osmclient/common/http.py | 2 + osmclient/common/package_tool.py | 2 +- osmclient/scripts/osm.py | 204 +++++++++++++++---------------- osmclient/sol005/client.py | 4 +- osmclient/sol005/osmrepo.py | 4 +- osmclient/sol005/vim.py | 16 ++- tox.ini | 4 +- 9 files changed, 134 insertions(+), 113 deletions(-) diff --git a/README.md b/README.md index be8b0d2..4b865c7 100644 --- a/README.md +++ b/README.md @@ -169,3 +169,13 @@ Then you can add the following to your $HOME/.bashrc file: . .bash_completion.d/osm-complete.sh ``` +## Future work + +- Create command classes for list and show operations +- Option `-c` for list and show operations to filter output and show only selected columns +- Option `-o ` to adapt output format (table, csv, yaml, json, jsonpath) +- Command ns-status to show the deployment status (RO) and configuration status (VCA) in an appealing format +- See how to deprecate commands and options: +- Evaluate the possibility to re-structure code to uniform all commands: `check`, `table_headers`, `run`, `output` + + diff --git a/osmclient/client.py b/osmclient/client.py index fe64f1c..a076fc4 100644 --- a/osmclient/client.py +++ b/osmclient/client.py @@ -24,6 +24,7 @@ from osmclient.sol005 import client as sol005client import logging import verboselogs +# pylint: disable=E1101 verboselogs.install() diff --git a/osmclient/common/http.py b/osmclient/common/http.py index fa5eae7..433edd0 100644 --- a/osmclient/common/http.py +++ b/osmclient/common/http.py @@ -17,6 +17,7 @@ from io import BytesIO import pycurl import json +import logging class Http(object): @@ -25,6 +26,7 @@ class Http(object): self._user = user self._password = password self._http_header = None + self._logger = logging.getLogger("osmclient") def set_http_header(self, header): self._http_header = header diff --git a/osmclient/common/package_tool.py b/osmclient/common/package_tool.py index abdcd3c..a2cbd87 100644 --- a/osmclient/common/package_tool.py +++ b/osmclient/common/package_tool.py @@ -81,7 +81,7 @@ class PackageTool(object): self._logger.debug("") # print("location: {}".format(osmclient.__path__)) file_loader = PackageLoader("osmclient") - env = Environment(loader=file_loader, autoscape=True) + env = Environment(loader=file_loader, autoescape=True) if package_type == "ns": template = env.get_template("nsd.yaml.j2" if not old else "nsd_old.yaml.j2") content = { diff --git a/osmclient/scripts/osm.py b/osmclient/scripts/osm.py index 2d9a231..397d932 100755 --- a/osmclient/scripts/osm.py +++ b/osmclient/scripts/osm.py @@ -96,14 +96,14 @@ def get_vim_name(vim_list, vim_id): def create_config(config_file, json_string): - ''' + """ Combines a YAML or JSON file with a JSON string into a Python3 structure It loads the YAML or JSON file 'cfile' into a first dictionary. It loads the JSON string into a second dictionary. Then it updates the first dictionary with the info in the second dictionary. If the field is present in both cfile and cdict, the field in cdict prevails. If both cfile and cdict are None, it returns an empty dict (i.e. {}) - ''' + """ config = {} if config_file: with open(config_file, "r") as cf: @@ -124,11 +124,6 @@ def create_config(config_file, json_string): envvar="OSM_HOSTNAME", help="hostname of server. " + "Also can set OSM_HOSTNAME in environment", ) -# @click.option('--sol005/--no-sol005', -# default=True, -# envvar='OSM_SOL005', -# help='Use ETSI NFV SOL005 API (default) or the previous SO API. ' + -# 'Also can set OSM_SOL005 in environment') @click.option( "--user", default=None, @@ -175,26 +170,6 @@ def create_config(config_file, json_string): help="user domain name for keystone authentication (default to None). " + "Also can set OSM_USER_DOMAIN_NAME in environment", ) -# @click.option('--so-port', -# default=None, -# envvar='OSM_SO_PORT', -# help='hostname of server. ' + -# 'Also can set OSM_SO_PORT in environment') -# @click.option('--so-project', -# default=None, -# envvar='OSM_SO_PROJECT', -# help='Project Name in SO. ' + -# 'Also can set OSM_SO_PROJECT in environment') -# @click.option('--ro-hostname', -# default=None, -# envvar='OSM_RO_HOSTNAME', -# help='hostname of RO server. ' + -# 'Also can set OSM_RO_HOSTNAME in environment') -# @click.option('--ro-port', -# default=None, -# envvar='OSM_RO_PORT', -# help='hostname of RO server. ' + -# 'Also can set OSM_RO_PORT in environment') @click.pass_context def cli_osm(ctx, **kwargs): global logger @@ -209,25 +184,7 @@ def cli_osm(ctx, **kwargs): exit(1) # Remove None values kwargs = {k: v for k, v in kwargs.items() if v is not None} - # if so_port is not None: - # kwargs['so_port']=so_port - # if so_project is not None: - # kwargs['so_project']=so_project - # if ro_hostname is not None: - # kwargs['ro_host']=ro_hostname - # if ro_port is not None: - # kwargs['ro_port']=ro_port sol005 = os.getenv("OSM_SOL005", True) - # if user is not None: - # kwargs['user']=user - # if password is not None: - # kwargs['password']=password - # if project is not None: - # kwargs['project']=project - # if all_projects: - # kwargs['all_projects']=all_projects - # if public is not None: - # kwargs['public']=public ctx.obj = client.Client(host=hostname, sol005=sol005, **kwargs) logger = logging.getLogger("osmclient") @@ -2363,7 +2320,7 @@ def ns_create( config, config_file, wait, - timeout + timeout, ): """creates a new NS instance""" logger.debug("") @@ -3130,9 +3087,15 @@ def pdu_delete(ctx, name, force): @click.option("--user", default=None, help="VIM username") @click.option("--password", default=None, help="VIM password") @click.option("--auth_url", default=None, help="VIM url") -@click.option("--tenant", "--project", "tenant", default=None, help="VIM tenant/project name") +@click.option( + "--tenant", "--project", "tenant", default=None, help="VIM tenant/project name" +) @click.option("--config", default=None, help="VIM specific config parameters") -@click.option("--config_file", default=None, help="VIM specific config parameters in YAML or JSON file") +@click.option( + "--config_file", + default=None, + help="VIM specific config parameters in YAML or JSON file", +) @click.option("--account_type", default="openstack", help="VIM type") @click.option("--description", default=None, help="human readable description") @click.option( @@ -3154,8 +3117,14 @@ def pdu_delete(ctx, name, force): "until the operation is completed, or timeout", ) @click.option("--vca", default=None, help="VCA to be used in this VIM account") -@click.option("--creds", default=None, help="credentials file (only applycable for GCP VIM type)") -@click.option("--prometheus_config_file", default=None, help="Prometheus configuration to get VIM data") +@click.option( + "--creds", default=None, help="credentials file (only applycable for GCP VIM type)" +) +@click.option( + "--prometheus_config_file", + default=None, + help="Prometheus configuration to get VIM data", +) @click.pass_context def vim_create( ctx, @@ -3173,7 +3142,7 @@ def vim_create( wait, vca, creds, - prometheus_config_file + prometheus_config_file, ): """creates a new VIM account""" logger.debug("") @@ -3200,7 +3169,9 @@ def vim_create( if creds: with open(creds, "r") as cf: vim_config["credentials"] = yaml.safe_load(cf.read()) - ctx.obj.vim.create(name, vim, vim_config, sdn_controller, sdn_port_mapping, wait=wait) + ctx.obj.vim.create( + name, vim, vim_config, sdn_controller, sdn_port_mapping, wait=wait + ) # except ClientException as e: # print(str(e)) # exit(1) @@ -3214,7 +3185,11 @@ def vim_create( @click.option("--auth_url", help="VIM url") @click.option("--tenant", help="VIM tenant name") @click.option("--config", help="VIM specific config parameters") -@click.option("--config_file", default=None, help="VIM specific config parameters in YAML or JSON file") +@click.option( + "--config_file", + default=None, + help="VIM specific config parameters in YAML or JSON file", +) @click.option("--account_type", help="VIM type") @click.option("--description", help="human readable description") @click.option( @@ -3236,8 +3211,14 @@ def vim_create( help="do not return the control immediately, but keep it " "until the operation is completed, or timeout", ) -@click.option("--creds", default=None, help="credentials file (only applycable for GCP VIM type)") -@click.option("--prometheus_config_file", default=None, help="Prometheus configuration to get VIM data") +@click.option( + "--creds", default=None, help="credentials file (only applycable for GCP VIM type)" +) +@click.option( + "--prometheus_config_file", + default=None, + help="Prometheus configuration to get VIM data", +) @click.pass_context def vim_update( ctx, @@ -3255,7 +3236,7 @@ def vim_update( sdn_port_mapping, wait, creds, - prometheus_config_file + prometheus_config_file, ): """updates a VIM account @@ -3290,7 +3271,9 @@ def vim_update( prometheus_config_dict = json.load(prometheus_file) vim["prometheus-config"] = prometheus_config_dict logger.info(f"VIM: {vim}, VIM config: {vim_config}") - ctx.obj.vim.update(name, vim, vim_config, sdn_controller, sdn_port_mapping, wait=wait) + ctx.obj.vim.update( + name, vim, vim_config, sdn_controller, sdn_port_mapping, wait=wait + ) # except ClientException as e: # print(str(e)) # exit(1) @@ -3924,7 +3907,19 @@ def sdnc_show(ctx, name): # help='do not return the control immediately, but keep it until the operation is completed, or timeout') @click.pass_context def k8scluster_add( - ctx, name, creds, version, vim, k8s_nets, init_helm2, init_helm3, init_jujubundle, description, namespace, wait, cni + ctx, + name, + creds, + version, + vim, + k8s_nets, + init_helm2, + init_helm3, + init_jujubundle, + description, + namespace, + wait, + cni, ): """adds a K8s cluster to OSM @@ -3940,9 +3935,11 @@ def k8scluster_add( cluster["vim_account"] = vim cluster["nets"] = yaml.safe_load(k8s_nets) if not (init_helm2 and init_jujubundle and init_helm3): - cluster["deployment_methods"] = {"helm-chart": init_helm2, - "juju-bundle": init_jujubundle, - "helm-chart-v3": init_helm3} + cluster["deployment_methods"] = { + "helm-chart": init_helm2, + "juju-bundle": init_jujubundle, + "helm-chart-v3": init_helm3, + } if description: cluster["description"] = description if namespace: @@ -4882,15 +4879,11 @@ def user_create(ctx, username, password, projects, project_role_mappings, domain multiple=True, help="remove role(s) in a project. Can be used several times: 'project,role1[,role2,...]'", ) -@click.option( - "--change_password", - "change_password", - help="user's current password" -) +@click.option("--change_password", "change_password", help="user's current password") @click.option( "--new_password", "new_password", - help="user's new password to update in expiry condition" + help="user's new password to update in expiry condition", ) @click.pass_context def user_update( @@ -5661,14 +5654,18 @@ def iterator_split(iterator, separators): if iterator[i] in separators: if i == first: continue - if (i - first < 2): - raise ClientException(f"Expected at least one argument after separator (possible separators: {separators}).") + if i - first < 2: + raise ClientException( + f"Expected at least one argument after separator (possible separators: {separators})." + ) list_of_lists.append(list(iterator[first:i])) first = i - if ((len(iterator) - first) < 2): - raise ClientException(f"Expected at least one argument after separator (possible separators: {separators}).") + if (len(iterator) - first) < 2: + raise ClientException( + f"Expected at least one argument after separator (possible separators: {separators})." + ) else: - list_of_lists.append(list(iterator[first:len(iterator)])) + list_of_lists.append(list(iterator[first : len(iterator)])) # logger.debug(f"List of lists: {list_of_lists}") return list_of_lists @@ -5679,9 +5676,9 @@ def process_common_heal_params(heal_vnf_dict, args): i = 0 while i < len(args): if args[i] == "--cause": - if (i+1 >= len(args)) or args[i+1].startswith("--"): + if (i + 1 >= len(args)) or args[i + 1].startswith("--"): raise ClientException("No cause was provided after --cause") - heal_vnf_dict["cause"] = args[i+1] + heal_vnf_dict["cause"] = args[i + 1] i = i + 2 continue if args[i] == "--run-day1": @@ -5698,18 +5695,22 @@ def process_common_heal_params(heal_vnf_dict, args): if "additionalParams" not in heal_vnf_dict: heal_vnf_dict["additionalParams"] = {} heal_vnf_dict["additionalParams"]["vdu"] = [] - if (i+1 >= len(args)) or args[i+1].startswith("--"): + if (i + 1 >= len(args)) or args[i + 1].startswith("--"): raise ClientException("No VDU ID was provided after --vdu") - heal_vnf_dict["additionalParams"]["vdu"].append({"vdu-id": args[i+1]}) + heal_vnf_dict["additionalParams"]["vdu"].append({"vdu-id": args[i + 1]}) current_item = "vdu" i = i + 2 continue if args[i] == "--count-index": if current_item == "vnf": - raise ClientException("Option --count-index only applies to VDU, not to VNF") - if (i+1 >= len(args)) or args[i+1].startswith("--"): + raise ClientException( + "Option --count-index only applies to VDU, not to VNF" + ) + if (i + 1 >= len(args)) or args[i + 1].startswith("--"): raise ClientException("No count index was provided after --count-index") - heal_vnf_dict["additionalParams"]["vdu"][-1]["count-index"] = int(args[i+1]) + heal_vnf_dict["additionalParams"]["vdu"][-1]["count-index"] = int( + args[i + 1] + ) i = i + 2 continue i = i + 1 @@ -5752,21 +5753,18 @@ def process_ns_heal_params(ctx, param, value): @cli_osm.command( name="ns-heal", short_help="heals (recreates) VNFs or VDUs of a NS instance", - context_settings=dict(ignore_unknown_options=True,) + context_settings=dict( + ignore_unknown_options=True, + ), ) @click.argument("ns_name") @click.argument( - 'args', + "args", nargs=-1, type=click.UNPROCESSED, callback=process_ns_heal_params, ) -@click.option( - "--timeout", - type=int, - default=None, - help="timeout in seconds" -) +@click.option("--timeout", type=int, default=None, help="timeout in seconds") @click.option( "--wait", default=False, @@ -5774,14 +5772,7 @@ def process_ns_heal_params(ctx, param, value): help="do not return the control immediately, but keep it until the operation is completed, or timeout", ) @click.pass_context -def ns_heal( - ctx, - ns_name, - args, - heal_params, - timeout, - wait -): +def ns_heal(ctx, ns_name, args, heal_params, timeout, wait): """heals (recreates) VNFs or VDUs of a NS instance NS_NAME: name or ID of the NS instance @@ -5812,11 +5803,15 @@ def ns_heal( vnf_filter = f"member-vnf-index-ref={vnf_id}" vnf_list = ctx.obj.vnf.list(ns=ns_name, filter=vnf_filter) if len(vnf_list) == 0: - raise ClientException(f"No VNF found in NS {ns_name} with filter {vnf_filter}") + raise ClientException( + f"No VNF found in NS {ns_name} with filter {vnf_filter}" + ) elif len(vnf_list) == 1: vnf["vnfInstanceId"] = vnf_list[0]["_id"] else: - raise ClientException(f"More than 1 VNF found in NS {ns_name} with filter {vnf_filter}") + raise ClientException( + f"More than 1 VNF found in NS {ns_name} with filter {vnf_filter}" + ) logger.debug(f"Heal dict:\n{yaml.safe_dump(heal_dict)}") check_client_version(ctx.obj, ctx.command.name) ctx.obj.ns.heal(ns_name, heal_dict, wait, timeout) @@ -5850,21 +5845,18 @@ def process_vnf_heal_params(ctx, param, value): @cli_osm.command( name="vnf-heal", short_help="heals (recreates) a VNF instance or the VDUs of a VNF instance", - context_settings=dict(ignore_unknown_options=True,) + context_settings=dict( + ignore_unknown_options=True, + ), ) @click.argument("vnf_name") @click.argument( - 'args', + "args", nargs=-1, type=click.UNPROCESSED, callback=process_vnf_heal_params, ) -@click.option( - "--timeout", - type=int, - default=None, - help="timeout in seconds" -) +@click.option("--timeout", type=int, default=None, help="timeout in seconds") @click.option( "--wait", default=False, @@ -6411,7 +6403,7 @@ def descriptor_translate(ctx, descriptor_file): def cli(): try: - cli_osm() + cli_osm() # pylint: disable=no-value-for-parameter exit(0) except pycurl.error as exc: print(exc) diff --git a/osmclient/sol005/client.py b/osmclient/sol005/client.py index b6f12b3..a69f3cc 100644 --- a/osmclient/sol005/client.py +++ b/osmclient/sol005/client.py @@ -131,7 +131,9 @@ class Client(object): token = json.loads(resp) if resp else None if token.get("message") == "change_password" and not pwd_change: - raise ClientException("Password Expired. Please update the password using change_password option") + raise ClientException( + "Password Expired. Please update the password using change_password option" + ) self._token = token["id"] if self._token is not None: diff --git a/osmclient/sol005/osmrepo.py b/osmclient/sol005/osmrepo.py index 85529de..0e6bb46 100644 --- a/osmclient/sol005/osmrepo.py +++ b/osmclient/sol005/osmrepo.py @@ -358,7 +358,9 @@ class OSMRepo(Repo): else: folder, descriptor_file = self.zip_extraction(path) folder = join(origin, folder) - self._logger.debug(f"Kind is an artifact (tar.gz). Folder: {folder}. Descriptor_file: {descriptor_file}") + self._logger.debug( + f"Kind is an artifact (tar.gz). Folder: {folder}. Descriptor_file: {descriptor_file}" + ) self._logger.debug("Opening descriptor file: {}".format(descriptor_file)) diff --git a/osmclient/sol005/vim.py b/osmclient/sol005/vim.py index 89c2db4..72a0822 100644 --- a/osmclient/sol005/vim.py +++ b/osmclient/sol005/vim.py @@ -70,7 +70,13 @@ class Vim(object): return "" def create( - self, name, vim_access, config={}, sdn_controller=None, sdn_port_mapping=None, wait=False + self, + name, + vim_access, + config={}, + sdn_controller=None, + sdn_port_mapping=None, + wait=False, ): vca_id = None @@ -127,7 +133,13 @@ class Vim(object): # raise ClientException("failed to create vim {} - {}".format(name, msg)) def update( - self, vim_name, vim_account, config, sdn_controller, sdn_port_mapping, wait=False + self, + vim_name, + vim_account, + config, + sdn_controller, + sdn_port_mapping, + wait=False, ): self._logger.debug("") self._client.get_token() diff --git a/tox.ini b/tox.ini index 8997616..604d3e2 100644 --- a/tox.ini +++ b/tox.ini @@ -34,7 +34,7 @@ parallel_show_output = true deps = black skip_install = true commands = - - black --check --diff osmclient/ + black --check --diff osmclient/ ####################################################################################### @@ -66,7 +66,7 @@ deps = {[testenv]deps} -r{toxinidir}/requirements-test.txt pylint commands = - - pylint -E osmclient + pylint -E osmclient ####################################################################################### -- 2.17.1