diff --git a/.github/workflows/main.yml b/.github/workflows/main.yml index 53462463..0802e82b 100644 --- a/.github/workflows/main.yml +++ b/.github/workflows/main.yml @@ -1,50 +1,80 @@ +# actions can be run locally using act and docker, on Fedora 37 also with podman, using: +# https://github.com/nektos/act +# sudo dnf install -y act-cli podman-docker +# act --bind --container-daemon-socket $XDG_RUNTIME_DIR/podman/podman.sock -W .github/workflows/main.yml + name: Unit tests +# Checks can be skipped by adding "skip-checks: true" to a commit message, +# or requested by adding "request-checks: true" if disabled by default for pushes: +# https://docs.github.com/en/pull-requests/collaborating-with-pull-requests/collaborating-on-repositories-with-code-quality-features/about-status-checks#skipping-and-requesting-checks-for-individual-commits on: [push, pull_request] +env: + PYTHONWARNINGS: "ignore" + PIP_ROOT_USER_ACTION: "ignore" # For local testing using act-cli + PIP_NO_WARN_SCRIPT_LOCATION: "0" # For local testing using act-cli jobs: - test_py2: - runs-on: ubuntu-20.04 + test: + strategy: + # max-parallel: 1 # See: https://github.com/xenserver/python-libs/pull/26#discussion_r1179482169 + matrix: + include: + - python-version: '3.11' + os: ubuntu-latest + - python-version: '3.10' + os: ubuntu-latest + - python-version: '2.7' + os: ubuntu-20.04 + - python-version: '3.9' + os: ubuntu-latest + - python-version: '3.8' + os: ubuntu-latest + - python-version: '3.7' + os: ubuntu-latest + - python-version: '3.6' + os: ubuntu-20.04 + runs-on: ${{ matrix.os }} steps: - - uses: actions/checkout@v2 + - uses: actions/checkout@v3 with: - fetch-depth: 0 - - name: Set up Python 2.7 - uses: actions/setup-python@v2 + fetch-depth: 0 # Needed by diff-cover to get the changed lines: origin/master..HEAD + - name: Set up Python ${{ matrix.python-version }} + uses: actions/setup-python@v4 with: - python-version: '2.7' + python-version: ${{ matrix.python-version }} - - name: Install dependencies + - name: Provide Pylint Summary and Report on the GitHub Action's info page + if: ${{ matrix.python-version == '3.10' }} run: | - python -m pip install --upgrade pip - pip install -r requirements-dev.txt - # FIXME: branding.py still has no permanent home - curl https://gist.github.com/ydirson/3c36a7e19d762cc529a6c82340894ccc/raw/5ca39f621b1feab813e171f535c1aad1bd483f1d/branding.py -O -L - pip install pyliblzma - pip install -e . - command -v xz + pip install -q pylint pandas tabulate 2>&1|grep -v root ||: # Hide warning for root in local container + tree=$GITHUB_SERVER_URL/$GITHUB_REPOSITORY/tree/$GITHUB_REF_NAME + [ -n "$GITHUB_STEP_SUMMARY" ] || GITHUB_STEP_SUMMARY=pylint-summary-table.md + ./gen_gh_pylint_report.py xcp $GITHUB_STEP_SUMMARY $tree - name: Test run: | - pytest --cov -rP - coverage xml - coverage html - coverage html -d htmlcov-tests --include="tests/*" - diff-cover --html-report coverage-diff.html coverage.xml + rm -f coverage.xml + pip -q install tox tox-gh-actions + tox --recreate - - name: Pylint - run: | - pylint --version - pylint --exit-zero xcp/ tests/ setup.py - pylint --exit-zero --msg-template="{path}:{line}: [{msg_id}({symbol}), {obj}] {msg}" xcp/ tests/ setup.py > pylint.txt - diff-quality --violations=pylint --html-report pylint-diff.html pylint.txt + - name: Upload coverage reports to Codecov + if: ${{ matrix.python-version == '3.11' }} + uses: codecov/codecov-action@v3 - uses: actions/upload-artifact@v3 + # Skip coverage upload during local actions testing using act-cli: + # For logging the contents of the context, see: + # https://docs.github.com/en/actions/learn-github-actions/contexts#example-printing-context-information-to-the-log + if: ${{ matrix.python-version == '3.11' && !startsWith(github.actor, 'nektos/act') }} with: name: Coverage and pylint reports path: | - coverage-diff.html - pylint-diff.html - htmlcov - htmlcov-tests + coverage.xml + .tox/py311-cov/log/htmlcov/ + .tox/py311-cov/log/htmlcov-tests/ + .tox/py311-cov/log/coverage-diff.html + .tox/py311-cov/log/pylint-diff.html + .tox/py311-cov/log/pylint.txt + .tox/py311-cov/log/.coverage diff --git a/README b/README deleted file mode 100644 index 550a1ea8..00000000 --- a/README +++ /dev/null @@ -1,5 +0,0 @@ -This repository contains utility classes. - -To contribute bug fixes, email them to the XenServer development mailing list -(xs-devel@lists.xenserver.org). - diff --git a/README.md b/README.md new file mode 100644 index 00000000..99ddeedb --- /dev/null +++ b/README.md @@ -0,0 +1,230 @@ +Common XenServer/XCP-ng Python classes +====================================== + +The `xcp` directory contains the Common XenServer and XCP-ng Python packages. +They are intented for use in XenServer and XCP-ng Dom0 only and deal with logging, +Hardware/PCI, networking, and other Dom0 tasks. + +The pip package name is `python-libs` which is also the rpm package name in XenServer. +XCP-ng packages it as [xcp-python-libs](https://github.com/xcp-ng-rpms/xcp-python-libs) +([koji](https://koji.xcp-ng.org/packageinfo?packageID=400)). + +It supports Python 2.7 and is currently in progress to get further fixes for >= 3.6. +It depends on `six`, and on Python 2.7, also `configparser` and `pyliblzma`. + +Pylint results from GitHub CI in GitHub Actions page +---------------------------------------------------- +A step of the GitHub workflow produces a browser-friendly `pylint` report: +From the [Actions tab](https://github.com/xenserver/python-libs/actions), +open a recent workflow run the latest and scroll down until you see the tables! + +Testing locally and in GitHub CI using tox +------------------------------------------ + +`pytest` runs tests. Checks by `pylint` and `mypy`. With `tox`, developers can +run the full test suite for Python 2.7 and 3.x. Unit tests are passing but there are + many Python3 issues which it does not uncover yet. + +> Intro: Managing a Project's Virtualenvs with tox - +> A comprehensive beginner's introduction to tox. +> https://www.seanh.cc/2018/09/01/tox-tutorial/ + +To run the tests for all supported and installed python versions, run: +```yaml +# The latest versions of tox need 1.11.0, so sensure that you have the latest py-1.11: +pip3 install 'py>=1.11.0' tox; tox +``` +You can run tox with just the python versions you have using `tox -e py27-test -e py3.11-mypy`. +The syntax is `-e py-[-factor2]` The currently supported factors +are: +- `test`: runs pytest +- `cov`: runs pytest --cov and generate XML and HTML reports in `.tox/py-cov/logs/` +- `mypy`: runs mypy +- `fox`: runs like `cov` but then opens the HTML reports in Firefox! + +The list of `virtualenvs` can be shown using this command: `tox -av -e py312-fox` +```yaml +using tox-3.28.0 from /usr/lib/python3.11/site-packages/tox/__init__.py (pid 157772) +default environments: +py27-test -> Run in a python2.7 virtualenv: pytest +py36-test -> Run in a python3.6 virtualenv: pytest +py37-test -> Run in a python3.7 virtualenv: pytest +py38-test -> Run in a python3.8 virtualenv: pytest +py39-test -> Run in a python3.9 virtualenv: pytest +py310-test -> Run in a python3.10 virtualenv: pytest +py311-mypy -> Run in a python3.11 virtualenv: mypy static analyis +py311-cov -> Run in a python3.11 virtualenv: generate coverage html reports +py312-test -> Run in a python3.12 virtualenv: pytest + +additional environments: +py312-fox -> Run in a python3.12 virtualenv: generate coverage html reports and open them in firefox +``` +If you have just one version of Python3, that will be enough, just use `tox -e py-test`. + +Installation of additional python versions for testing different versions: +- Fedora 37: `sudo dnf install tox` installs all Python versions, even 3.12a7. +- On Ubuntu, the deadsnakes/ppa is broken(except for 3.12), so conda or pyenv has to be used. + For full instructions, see https://realpython.com/intro-to-pyenv/, E.g install on Ubuntu: + ```yaml + sudo apt-get install -y build-essential libssl-dev zlib1g-dev libbz2-dev + libreadline-dev libsqlite3-dev xz-utils libffi-dev liblzma-dev + curl https://pyenv.run | bash # and add the displayed commands to .bashrc + pyenv install 3.{6,7,8,9} && pyenv local 3.{6,7,8,9} # builds and adds them + ``` +- Note: `virtualenv-20.22` broke creating the `py27` venv with tox, at least in some setups. + As a workaround, downgrade it to 20.21 if that happens: `pip3 install -U 'virtualenv<20.22'` +- For testing on newer Ubuntu hosts which have `python2-dev`, but not `pip2`, install `pip2` this way: + ```json + curl https://bootstrap.pypa.io/pip/2.7/get-pip.py --output get-pip.py;sudo python2 get-pip.py + ``` + +Static analysis using mypy and pyright +-------------------------------------- +The preconditions for using static analysis with `mypy` (which passes now but has +only a few type comments) and `pyright` are present now and `mypy` is enabled in `tox` +which runs the tests in GitHub CI as well. But of course, because they code is largely +still not yet typed, no strict checks can be enabled so far. However, every checker +which is possible now, is enabled. + +Checking the contents of untyped functions is enabled for all but four modules which +would need more work. Look for `check_untyped_defs = false` in `pytproject.toml`. + +The goal or final benefit would be to have it to ensure internal type correctness +and code quality but also to use static analysis to check the interoperability with +the calling code. + +Type annotations: Use Type comments for now! +-------------------------------------------- +Python2.7 can't support the type annotation syntax, but until all users are migrated, +annotations in comments (type comments) can be used. They are supported by +tools like `mypy` and `pyright` (VS Code): + +Quoting from https://stackoverflow.com/questions/53306458/python-3-type-hints-in-python-2: + +> Function annotations were introduced in [PEP 3107](https://www.python.org/dev/peps/pep-3107/) for Python 3.0. The usage of annotations as type hints was formalized in in [PEP 484](https://www.python.org/dev/peps/pep-0484/) for Python 3.5+. +> +> Any version before 3.0 then will not support the syntax you are using for type hints at all. However, PEP 484 [offers a workaround](https://www.python.org/dev/peps/pep-0484/#suggested-syntax-for-python-2-7-and-straddling-code), which some editors may choose to honor. In your case, the hints would look like this: +```py +def get_default_device(use_gpu=True): + # type: (bool) -> cl.Device + ... +``` +> or more verbosely, +```py +def get_default_device(use_gpu=True # type: bool + ): + # type: (...) -> cl.Device + ... +``` +> The PEP explicitly states that this form of type hinting should work for any version of Python. + +As proof, these examples show how the comment below triggers the checks: +```diff +--- a/xcp/xmlunwrap.py ++++ b/xcp/xmlunwrap.py +@@ -29,1 +29,2 @@ class XmlUnwrapError(Exception): + def getText(nodelist): ++ # type:(Element) -> str +``` +mypy: +```py +$ mypy xcp/xmlunwrap.py +xcp/xmlunwrap.py:31: error: Name "Element" is not defined +xcp/xmlunwrap.py:38: error: Incompatible return value type (got "bytes", expected "str") +``` +pyright (used by VS Code by default): +```py +$ pyright xcp/xmlunwrap.py|sed "s|$PWD/||" +... +pyright 1.1.295 +xcp/xmlunwrap.py + xcp/xmlunwrap.py:32:13 - error: "Element" is not defined (reportUndefinedVariable) + xcp/xmlunwrap.py:38:12 - error: Expression of type "Unknown | bytes" cannot be assigned to return type "str" +   Type "Unknown | bytes" cannot be assigned to type "str" +     "bytes" is incompatible with "str" (reportGeneralTypeIssues) + xcp/xmlunwrap.py:81:38 - error: Argument of type "Unknown | None" cannot be assigned to parameter "default" of type "str" in function "getStrAttribute" +   Type "Unknown | None" cannot be assigned to type "str" +     Type "None" cannot be assigned to type "str" (reportGeneralTypeIssues) +3 errors, 0 warnings, 0 informations +Completed in 0.604sec +``` +See https://github.com/xenserver/python-libs/pull/23 for the context of this example. + +Special open TODOs: +------------------- + +Charset encoding/string handling: +* With Python3, `read()` on files `open()`ed without specifying binary mode will attempt + to decode the data into the Python3 Unicode string type, which will fail for all + binary data. Thus all `open()` calls which might open binary files have to be converted + to binary mode by default unless the caller is sure he is opening an ASCII file, + even then, enabling an error handle to handle decoding errors is recommended. +* With Python3, the `stdin`, `stdout` and `stderr` pipes for `Popen()` default to + `bytes`(binary mode.) Binary mode is much safer because it foregoes the encode/decode. The existing users need to be able to enable text mode (when safe, it will attempt + to decode and encode!) or preferably be able to use bytes (which is the type behind Python2 strings too) instead. See these PRs for details: + * https://github.com/xenserver/python-libs/pull/22 + * https://github.com/xenserver/python-libs/pull/23 + * https://github.com/xenserver/python-libs/pull/24 + * What's more: When code is called from a xapi plugin (such as ACK), when such code + attempts to read text files like the `pciids` file, and there is a Unicode char + it int, and the locale is not set up to be UTF-8 (because xapi plugins are started + from xapi), the UTF-8 decoder has to be explicitly enabled for these files, + bese by adding `encoding="utf-8"` to the arguments of these specific `open()` calls, + to have valid Unicode text strings, e.g. `xcp.pci`, for regular text processing. + * TODO: More to be opened for all remaining `open()` and `Popen()` users, + as well as ensuring that users of `urllib` are able to work with they bytes + it returns (there is no option to use text mode, data may be gzip-encoded!) + +Users +----- + +* https://github.com/xenserver/host-installer + * /opt/xensource/installer/ (has copies of `cpiofile.py`, `repository.py` (with `accessor.py`) +* https://github.com/xcp-ng-rpms/host-upgrade-plugin ([koji](https://koji.xcp-ng.org/packageinfo?packageID=104)): + * /etc/xapi.d/plugins/prepare_host_upgrade.py +* https://github.com/xapi-project/xen-api (`xapi-core.rpm` and `xenopsd.rpm`) + * /etc/xapi.d/extensions/pool_update.apply + * /etc/xapi.d/extensions/pool_update.precheck + * /etc/xapi.d/plugins/disk-space + * /etc/xapi.d/plugins/install-supp-pack + * /opt/xensource/libexec/host-display + * /opt/xensource/libexec/mail-alarm + * /opt/xensource/libexec/usb_reset.py + * /opt/xensource/libexec/usb_scan.py + * /usr/libexec/xenopsd/igmp_query_injector.py +* xenserver-release-config/[xcp-ng-release-config](https://koji.xcp-ng.org/rpminfo?rpmID=10250) + * /opt/xensource/libexec/fcoe_driver + * /opt/xensource/libexec/xen-cmdline +* https://github.com/xcp-ng-rpms/interface-rename: + * /etc/sysconfig/network-scripts/interface-rename.py + * /opt/xensource/bin/interface-rename +* pvsproxy (Proprietary) + * /usr/libexec/xapi-storage-script/volume/org.xen.xapi.storage.tmpfs/memoryhelper.py +* https://github.com/xenserver/linux-guest-loader (not installed by default anymore) + * /opt/xensource/libexec/eliloader.py +* https://github.com/xcp-ng-rpms/vcputune + * /opt/xensource/bin/host-cpu-tune +* The ACK xenapi plugin see: https://github.com/xenserver/python-libs/pull/21 + +Verification: +```ps +# rpm -qf $(grep -r import /usr/libexec/ /usr/bin /etc/xapi.d/ /opt/xensource/|grep xcp|cut -d: -f1|grep -v Binary) --qf '%{name}\n'|sort -u|tee xcp-python-libs-importers.txt +host-upgrade-plugin +interface-rename +pvsproxy +vcputune +xapi-core +xenopsd +xenserver-release-config +# grep -s import $(rpm -ql xapi-core)|grep xcp|cut -d: -f1 +/etc/xapi.d/extensions/pool_update.apply +/etc/xapi.d/extensions/pool_update.precheck +/etc/xapi.d/plugins/disk-space +/etc/xapi.d/plugins/disk-space +/etc/xapi.d/plugins/install-supp-pack +/opt/xensource/libexec/host-display +/opt/xensource/libexec/mail-alarm +/opt/xensource/libexec/usb_reset.py +/opt/xensource/libexec/usb_scan.py +``` + diff --git a/branding.py b/branding.py new file mode 100644 index 00000000..88914002 --- /dev/null +++ b/branding.py @@ -0,0 +1,36 @@ +BRAND_CONSOLE_URL = "https://xcp-ng.org" +BRAND_CONSOLE = "XCP-ng Center" +BRAND_GUEST_SHORT = "VM" +BRAND_GUESTS_SHORT = "VMs" +BRAND_GUESTS = "Virtual Machines" +BRAND_GUEST = "Virtual Machine" +BRAND_SERVERS = "XCP-ng Hosts" +BRAND_SERVER = "XCP-ng Host" +BRAND_VDI = "" +COMPANY_DOMAIN = "xcp-ng.org" +COMPANY_NAME_LEGAL = "Open Source" +COMPANY_NAME = "Open Source" +COMPANY_NAME_SHORT = "Open Source" +COMPANY = "Open Source" +COMPANY_PRODUCT_BRAND = "XCP-ng" +COMPANY_WEBSITE = "https://xcp-ng.org" +COPYRIGHT_YEARS = "2018-2022" +ISO_PV_TOOLS_COPYRIGHT = "XCP-ng" +ISO_PV_TOOLS_LABEL = "XCP-ng VM Tools" +ISO_PV_TOOLS_PUBLISHER = "XCP-ng" +PLATFORM_MAJOR_VERSION = "3" +PLATFORM_MICRO_VERSION = "1" +PLATFORM_MINOR_VERSION = "2" +PLATFORM_NAME = "XCP" +PLATFORM_ORGANISATION = "xen.org" +PLATFORM_VERSION = "3.2.1" +PLATFORM_WEBSITE = "www.xen.org" +PRODUCT_BRAND = "XCP-ng" +PRODUCT_BRAND_DASHED = "XCP-ng" +PRODUCT_MAJOR_VERSION = "8" +PRODUCT_MICRO_VERSION = "1" +PRODUCT_MINOR_VERSION = "2" +PRODUCT_NAME = "xenenterprise" +PRODUCT_VERSION = "8.2.1" +PRODUCT_VERSION_TEXT = "8.2" +PRODUCT_VERSION_TEXT_SHORT = "8.2" diff --git a/gen_gh_pylint_report.py b/gen_gh_pylint_report.py new file mode 100755 index 00000000..7b60b450 --- /dev/null +++ b/gen_gh_pylint_report.py @@ -0,0 +1,178 @@ +#!/usr/bin/env python3 +import json +import os +import sys +from glob import glob +from io import StringIO, TextIOWrapper +from typing import List + +from pylint.lint import Run # type: ignore +from pylint.reporters import JSONReporter # type: ignore + +import pandas as pd + + +def del_dict_keys(r, *args): + for arg in args: + r.pop(arg, None) + + +def cleanup_results_dict(r, sym): + del_dict_keys( + r, + "module", + "column", + "endColumn", + "message-id", + "endLine", + "type", + "line", + ) + r["symbol"] = sym[:32] + r["message"] = r["message"][:96] + try: + dotpos = r["obj"].rindex(".") + 1 + except ValueError: + dotpos = 0 + r["obj"] = r["obj"][dotpos:][:16] + + +suppress_msg = ["Consi", "Unnec", "Unuse", "Use l", "Unkno", "Unrec", "Insta"] +suppress_sym = [ + "attribute-defined-outside-init", + "bare-except", + "broad-exception-raised", + # "duplicate-except", + "super-init-not-called", +] +notice_syms = [ + "fixme", + "no-member", + "unexpected-keyword-arg", + "assignment-from-no-return", +] + + +def filter_results(r): + msg = r["message"] + typ = r["type"] + if typ in ["convention", "refactor"] or not msg: + return None, None, None + sym = r["symbol"] + return ( + (None, None, None) + if sym in suppress_sym or msg[:5] in suppress_msg + else (typ, sym, msg) + ) + + +def pylint_project(module_path: str, branch_url: str, errorlog: TextIOWrapper): + pylint_options: List[str] = [] + pylint_overview = [] + pylint_results = [] + glob_pattern = os.path.join(module_path, "**", "*.py") + score_sum = 0.0 + smell_sum = 0 + for filepath in glob(glob_pattern, recursive=True): + filename = filepath.rsplit("/", maxsplit=1)[-1] + if filename in ["__init__.py", "pylintrc"]: + continue + reporter_buffer = StringIO() + results = Run( + [filepath] + pylint_options, + reporter=JSONReporter(reporter_buffer), + do_exit=False, + ) + score = results.linter.stats.global_note + file_results = json.loads(reporter_buffer.getvalue()) + if not file_results: + continue + filtered_file_results = [] + filtered_messages = {} + linktext = filename.split(".")[0] + for r in file_results: + typ, sym, msg = filter_results(r) + if msg is None: + continue + print(typ, sym, msg) + if sym in notice_syms: + typ = "notice" + else: + filtered_messages[sym] = 0 + errorlog.write( + f"::{typ} file={filepath},line={r['line']},endLine={r['endLine']}," + f"title={sym}::{msg}\n" + ) + r["path"] = f"[{linktext}]({branch_url}/{filepath}#L{r['line']})" + cleanup_results_dict(r, sym) + filtered_file_results.append(r) + + pylint_results.extend(filtered_file_results) + smells = len(filtered_file_results) + smell_sum += smells + score_sum += score + + pylint_overview.append( + { + "filepath": f"[{filepath[4:]}]({branch_url}/{filepath})", + "smells": smells, + "symbols": " ".join(filtered_messages.keys()), + "score": round(score, 3), + } + ) + avg_score = score_sum / len(pylint_overview) + pylint_overview.append( + { + "filepath": "total", + "smells": smell_sum, + "symbols": "", + "score": round(avg_score, 3), + } + ) + return pd.DataFrame(pylint_overview), pd.DataFrame(pylint_results) # , avg_score + + +def main(module_dir: str, output_file: str, branch_url: str, errorlog_file: str): + """Send pylint errors, warnings, notices to stdout. Github show show 10 of each + + Args: + module_dir (str): subdirectory of the module, e.g. "xcp" + output_file (str): output file path for the markdown summary table + branch_url (str): _url of the branch for file links in the summary table + """ + with open(errorlog_file, "a", encoding="utf-8") as errors: + panda_overview, panda_results = pylint_project(module_dir, branch_url, errors) + + # print("### Overview") + # print(panda_overview.to_markdown()) + # print("\n### Results") + # print(panda_results.to_markdown()) + + # Write the panda dable to a markdown output file: + summary_file = output_file or os.environ.get("GITHUB_STEP_SUMMARY") + scriptname = __file__.rsplit("/", maxsplit=1)[-1] + mylink = f"[{scriptname}]({branch_url}/{scriptname})" + if summary_file: + with open(summary_file, "w", encoding="utf-8") as fp: + fp.write( + f"### PyLint breakdown from {mylink} on **xcp/\\*\\*/*.py**\n" + ) + fp.write(panda_overview.to_markdown()) + + fp.write( + f"\n### PyLint results from {mylink} on **xcp/\\*\\*/*.py**\n" + ) + fp.write(panda_results.to_markdown()) + + +if __name__ == "__main__": + default_branch_url = "https://github.com/xenserver/python-libs/blob/master" + default_error_file = "/dev/stderr" + + py_module_dir = sys.argv[1] if len(sys.argv) > 1 else "xcp" + gh_output_file = sys.argv[2] if len(sys.argv) > 2 else "pylint-summary-table.md" + gh_branch_url = sys.argv[3] if len(sys.argv) > 3 else default_branch_url + errorlog_path = sys.argv[4] if len(sys.argv) > 4 else default_error_file + + print(py_module_dir, gh_output_file, gh_branch_url, errorlog_path) + main(py_module_dir, gh_output_file, gh_branch_url, errorlog_path) diff --git a/pyproject.toml b/pyproject.toml new file mode 100644 index 00000000..7b34526b --- /dev/null +++ b/pyproject.toml @@ -0,0 +1,131 @@ +[project] +# https://packaging.python.org/en/latest/specifications/declaring-project-metadata/ +name = "python-libs" +version = "3.0.0" +description = "Common XCP-ng Python classes" +requires-python = ">=2.7" +license = {file = "LICENSE"} +keywords = ["xcp-ng", "xen-project", "libraries"] +authors = [ + {name = "Simon Rowe"}, + {name = "Andrew Cooper"}, + {name = "Yann Dirson"}, +] +maintainers = [ + {name = "Ross Lagerwall"}, + {name = "Pau Ruiz Safont"}, + {name = "Bernhard Kaindl"}, +] +readme = "README.md" +classifiers = [ + "Development Status :: 5 - Production/Stable", + "Operating System :: POSIX :: Linux", + "Programming Language :: Python :: 2.7", + "Programming Language :: Python :: 3.6", + "Programming Language :: Python :: 3.7", + "Programming Language :: Python :: 3.8", + "Programming Language :: Python :: 3.9", + "Programming Language :: Python :: 3.10", + "Programming Language :: Python :: 3.11", + "Programming Language :: Python :: 3.12", + "Programming Language :: Python :: Implementation :: CPython", + "Topic :: System :: Hardware", + "Topic :: System :: Installation/Setup", + "Topic :: System :: Networking", + "Topic :: System :: Systems Administration", + "Topic :: Software Development :: Libraries :: Python Modules" +] + +[project.optional-dependencies] +coverage = ["coverage", "diff_cover", "pytest-cov"] + +[project.urls] +homepage = "https://github.com/xenserver/python-libs/" +repository = "https://github.com/xenserver/python-libs/" + +[build-system] +requires = ["setuptools >= 38.6.0", "wheel"] +build-backend = "setuptools.build_meta" + +[tool.mypy] +pretty = true +show_error_context = true +error_summary = true +files = ["xcp", "tests/test_*.py", "stubs"] +python_version = "3.6" +warn_redundant_casts = true +warn_return_any = true +warn_unreachable = true +warn_unused_configs = true +disallow_any_unimported = true +disallow_any_explicit = true +disallow_any_generics = true +disallow_subclassing_any = true +show_error_codes = true +strict_equality = true +# Check the contents of untyped functions in all modules by default: +check_untyped_defs = true + +# xcp.cmd is to be fixed in PR #22: +[[tool.mypy.overrides]] +module = ["xcp.cmd"] +disable_error_code = ["operator", "comparison-overlap"] + +# xcp.accessor is be fixed with #24: +[[tool.mypy.overrides]] +module = ["xcp.accessor"] +disable_error_code = "union-attr" + +# To be fixed with #24: +[[tool.mypy.overrides]] +module = ["xcp.net.biosdevname", "xcp.net.ifrename.logic"] +disable_error_code = ["var-annotated", "no-any-return"] + +# xcp.net.ip should be is to be fixed in PR #22, but the ip output parser works anyway: +[[tool.mypy.overrides]] +module = ["xcp.net.ip"] +disable_error_code = ["arg-type", "comparison-overlap"] + +# The blame list of modules with various errors/warnings in their untyped defs, +# it shuts up 65 mypy errors, most are in cpiofile: +[[tool.mypy.overrides]] +module = [ + "xcp.pci", # xcp.pci should be fixed by PR #22 + "xcp.cpiofile", + "xcp.repository", + "xcp.bootloader", +] +check_untyped_defs = false # enable to see the blame list +disable_error_code = ["var-annotated", "unreachable"] + +# Most of these should be easily fixable by adding type annotations as comments(PEP484): + +[[tool.mypy.overrides]] +module = ["tests.test_pci"] +disable_error_code = ["no-any-return"] + +[[tool.mypy.overrides]] +module = ["tests.test_mac"] +disable_error_code = ["var-annotated"] + +[[tool.mypy.overrides]] +module = ["tests.test_ifrename_logic"] +disable_error_code = ["attr-defined", "no-any-return"] + +[[tool.mypy.overrides]] +module = ["tests.test_bootloader"] +disable_error_code = ["no-any-return", "union-attr"] + +[[tool.mypy.overrides]] +module = ["tests.test_ifrename_logic"] +disable_error_code = ["attr-defined", "no-any-return"] + +# Due to special cases for unicode handline in Python2 which are not reached by Python3: +[[tool.mypy.overrides]] +module = "xcp.xmlunwrap" +warn_unreachable = false + +[tool.coverage.run] +# The coverage-comment-action needs a .converage file with relative path names: +# https://github.com/py-cov-action/python-coverage-comment-action#setup +relative_files = true \ No newline at end of file diff --git a/requirements-dev.txt b/requirements-dev.txt index b2799686..99d92b25 100644 --- a/requirements-dev.txt +++ b/requirements-dev.txt @@ -1,10 +1,14 @@ # necessary dev/test tasks pylint -coverage +coverage[toml] diff_cover mock pytest pytest-cov +parameterized # dependencies also in setup.py until they can be used six -future + +# python-2.7 only +configparser ; python_version < "3.0" +pyliblzma ; python_version < "3.0" diff --git a/setup.py b/setup.py index 08c8200d..6d4b82b6 100644 --- a/setup.py +++ b/setup.py @@ -25,9 +25,10 @@ SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF SUCH DAMAGE. """ -from distutils.core import setup +from setuptools import setup setup(name='python-libs', + version='3.0.0', description='Common XenServer Python classes', packages=['xcp', 'xcp.net', @@ -36,6 +37,5 @@ requires=[ 'branding', 'six', - 'future', ], ) diff --git a/stubs/branding.pyi b/stubs/branding.pyi new file mode 100644 index 00000000..0a3d72c4 --- /dev/null +++ b/stubs/branding.pyi @@ -0,0 +1,43 @@ +""" +python type stup for automatic type checking using mypy, prright et al. + +Generated from tests/branding.py using mypy's stubgen tool: +stubgen tests/branding.py +https://mypy.readthedocs.io/en/stable/stubgen.html?highlight=paths#automatic-stub-generation-stubgen +""" +BRAND_CONSOLE_URL: str +BRAND_CONSOLE: str +BRAND_GUEST_SHORT: str +BRAND_GUESTS_SHORT: str +BRAND_GUESTS: str +BRAND_GUEST: str +BRAND_SERVERS: str +BRAND_SERVER: str +BRAND_VDI: str +COMPANY_DOMAIN: str +COMPANY_NAME_LEGAL: str +COMPANY_NAME: str +COMPANY_NAME_SHORT: str +COMPANY: str +COMPANY_PRODUCT_BRAND: str +COMPANY_WEBSITE: str +COPYRIGHT_YEARS: str +ISO_PV_TOOLS_COPYRIGHT: str +ISO_PV_TOOLS_LABEL: str +ISO_PV_TOOLS_PUBLISHER: str +PLATFORM_MAJOR_VERSION: str +PLATFORM_MICRO_VERSION: str +PLATFORM_MINOR_VERSION: str +PLATFORM_NAME: str +PLATFORM_ORGANISATION: str +PLATFORM_VERSION: str +PLATFORM_WEBSITE: str +PRODUCT_BRAND: str +PRODUCT_BRAND_DASHED: str +PRODUCT_MAJOR_VERSION: str +PRODUCT_MICRO_VERSION: str +PRODUCT_MINOR_VERSION: str +PRODUCT_NAME: str +PRODUCT_VERSION: str +PRODUCT_VERSION_TEXT: str +PRODUCT_VERSION_TEXT_SHORT: str diff --git a/stubs/parameterized/__init__.pyi b/stubs/parameterized/__init__.pyi new file mode 100644 index 00000000..aed931d0 --- /dev/null +++ b/stubs/parameterized/__init__.pyi @@ -0,0 +1 @@ +from .parameterized import param as param, parameterized as parameterized, parameterized_class as parameterized_class diff --git a/stubs/parameterized/parameterized.pyi b/stubs/parameterized/parameterized.pyi new file mode 100644 index 00000000..23dba7d7 --- /dev/null +++ b/stubs/parameterized/parameterized.pyi @@ -0,0 +1,76 @@ +from _typeshed import Incomplete +from collections import OrderedDict as MaybeOrderedDict +from typing import NamedTuple + +# MaybeOrderedDict = dict + +class SkipTest(Exception): ... + +PY3: Incomplete +PY2: Incomplete +PYTEST4: Incomplete + +class InstanceType: ... + +lzip: Incomplete +text_type = str +string_types: Incomplete +bytes_type = bytes + +def make_method(func, instance, type): ... +def to_text(x): ... + +class CompatArgSpec(NamedTuple): + args: Incomplete + varargs: Incomplete + keywords: Incomplete + defaults: Incomplete + +def getargspec(func): ... +def skip_on_empty_helper(*a, **kw) -> None: ... +def reapply_patches_if_need(func): ... +def delete_patches_if_need(func) -> None: ... + +class _param(NamedTuple): + args: Incomplete + kwargs: Incomplete + +class param(_param): + def __new__(cls, *args, **kwargs): ... + @classmethod + def explicit(cls, args: Incomplete | None = ..., kwargs: Incomplete | None = ...): ... + @classmethod + def from_decorator(cls, args): ... + +class QuietOrderedDict(MaybeOrderedDict): ... # type: ignore + +def parameterized_argument_value_pairs(func, p): ... +def short_repr(x, n: int = ...): ... +def default_doc_func(func, num, p): ... +def default_name_func(func, num, p): ... +def set_test_runner(name) -> None: ... +def detect_runner(): ... + +class parameterized: + get_input: Incomplete + doc_func: Incomplete + skip_on_empty: Incomplete + def __init__(self, input, doc_func: Incomplete | None = ..., skip_on_empty: bool = ...) -> None: ... + def __call__(self, test_func): ... + def param_as_nose_tuple(self, test_self, func, num, p): ... + def assert_not_in_testcase_subclass(self) -> None: ... + @classmethod + def input_as_callable(cls, input): ... + @classmethod + def check_input_values(cls, input_values): ... + @classmethod + def expand(cls, input, name_func: Incomplete | None = ..., doc_func: Incomplete | None = ..., skip_on_empty: bool = ..., **legacy): ... + @classmethod + def param_as_standalone_func(cls, p, func, name): ... + @classmethod + def to_safe_name(cls, s): ... + +def parameterized_class(attrs, input_values: Incomplete | None = ..., class_name_func: Incomplete | None = ..., classname_func: Incomplete | None = ...): ... +def unwrap_mock_patch_func(f): ... +def get_class_name_suffix(params_dict): ... +def default_class_name_func(cls, num, params_dict): ... diff --git a/stubs/parameterized/test.pyi b/stubs/parameterized/test.pyi new file mode 100644 index 00000000..aab4e91f --- /dev/null +++ b/stubs/parameterized/test.pyi @@ -0,0 +1,100 @@ +from .parameterized import PY2 as PY2, PY3 as PY3, PYTEST4 as PYTEST4, SkipTest as SkipTest, detect_runner as detect_runner, param as param, parameterized as parameterized, parameterized_argument_value_pairs as parameterized_argument_value_pairs, parameterized_class as parameterized_class, short_repr as short_repr +from _typeshed import Incomplete +from unittest import TestCase + +def assert_contains(haystack, needle) -> None: ... + +runner: Incomplete +UNITTEST: Incomplete +NOSE2: Incomplete +PYTEST: Incomplete +SKIP_FLAGS: Incomplete +missing_tests: Incomplete + +def expect(skip, tests: Incomplete | None = ...) -> None: ... + +test_params: Incomplete + +def test_naked_function(foo, bar: Incomplete | None = ...) -> None: ... + +class TestParameterized: + def test_instance_method(self, foo, bar: Incomplete | None = ...) -> None: ... + +class TestSetupTeardown: + stack: Incomplete + actual_order: str + def setUp(self) -> None: ... + def tearDown(self) -> None: ... + def test_setup(self, count, *a) -> None: ... + +def custom_naming_func(custom_tag, kw_name): ... + +class TestParameterizedExpandWithMockPatchForClass(TestCase): + def test_one_function_patch_decorator(self, foo, mock_umask, mock_getpid) -> None: ... + def test_multiple_function_patch_decorator(self, foo, bar, mock_umask, mock_fdopen, mock_getpid) -> None: ... + +class TestParameterizedExpandWithNoExpand: + def test_patch_class_no_expand(self, foo, bar, mock_umask, mock_getpid) -> None: ... + +class TestParameterizedExpandWithNoMockPatchForClass(TestCase): + def test_one_function_patch_decorator(self, foo, mock_umask) -> None: ... + def test_multiple_function_patch_decorator(self, foo, bar, mock_umask, mock_fdopen) -> None: ... + +class TestParameterizedExpandWithNoMockPatchForClassNoExpand: + def test_patch_no_expand(self, foo, bar, mock_umask) -> None: ... + +def test_mock_patch_standalone_function(foo, mock_umask) -> None: ... + +class TestParamerizedOnTestCase(TestCase): + def test_on_TestCase(self, foo, bar: Incomplete | None = ...) -> None: ... + def test_on_TestCase2(self, foo, bar: Incomplete | None = ...) -> None: ... + +class TestParameterizedExpandDocstring(TestCase): + def test_custom_doc_func(self, foo, bar: Incomplete | None = ...) -> None: ... + def test_single_line_docstring(self, foo) -> None: ... + def test_empty_docstring(self, foo) -> None: ... + def test_multiline_documentation(self, foo) -> None: ... + def test_unicode_docstring(self, foo) -> None: ... + def test_default_values_get_correct_value(self, foo, bar: int = ...) -> None: ... + def test_with_leading_newline(self, foo, bar: int = ...) -> None: ... + +def test_warns_when_using_parameterized_with_TestCase() -> None: ... +def test_helpful_error_on_invalid_parameters() -> None: ... +def test_helpful_error_on_empty_iterable_input() -> None: ... +def test_skip_test_on_empty_iterable() -> None: ... +def test_helpful_error_on_empty_iterable_input_expand() -> None: ... +def test_wrapped_iterable_input(foo) -> None: ... +def test_helpful_error_on_non_iterable_input(): ... +def tearDownModule() -> None: ... +def test_old_style_classes() -> None: ... + +class TestOldStyleClass: + def test_old_style_classes(self, param) -> None: ... + +def test_parameterized_argument_value_pairs(func_params, p, expected) -> None: ... +def test_short_repr(input, expected, n: int = ...) -> None: ... +def test_with_docstring(input) -> None: ... + +cases_over_10: Incomplete + +def test_cases_over_10(input, expected) -> None: ... + +class TestParameterizedClass(TestCase): + def test_method_a(self) -> None: ... + def test_method_b(self) -> None: ... + def testCamelCaseMethodC(self) -> None: ... + +class TestNamedParameterizedClass(TestCase): + def test_method(self) -> None: ... + +class TestParameterizedClassDict(TestCase): + foo: int + bar: str + def setUp(self) -> None: ... + def tearDown(self) -> None: ... + def test_method(self) -> None: ... + +class TestUnicodeDocstring: + def test_with_docstring(self, param) -> None: ... + +def test_missing_argument_error() -> None: ... diff --git a/tests/__init__.py b/tests/__init__.py new file mode 100644 index 00000000..02b47bd0 --- /dev/null +++ b/tests/__init__.py @@ -0,0 +1,2 @@ +# Needed for mypy to see the tests as module in which we can suppress warnings +# using configuration in pyproject.toml. diff --git a/tests/data/repo/boot/isolinux/mboot.c32 b/tests/data/repo/boot/isolinux/mboot.c32 new file mode 100644 index 00000000..3f5b2fe5 Binary files /dev/null and b/tests/data/repo/boot/isolinux/mboot.c32 differ diff --git a/tests/test_accessor.py b/tests/test_accessor.py index ade787e6..0e8441a5 100644 --- a/tests/test_accessor.py +++ b/tests/test_accessor.py @@ -1,21 +1,46 @@ import unittest +import hashlib +from tempfile import NamedTemporaryFile + +from parameterized import parameterized_class import xcp.accessor +@parameterized_class([{"url": "file://tests/data/repo/"}, + {"url": "https://updates.xcp-ng.org/netinstall/8.2.1"}]) class TestAccessor(unittest.TestCase): - def test_http(self): - raise unittest.SkipTest("comment out if you really mean it") - a = xcp.accessor.createAccessor("https://updates.xcp-ng.org/netinstall/8.2.1", True) - a.start() - self.assertTrue(a.access('.treeinfo')) + url = "" + + def setup_accessor(self, arg0): + result = xcp.accessor.createAccessor(self.url, True) + result.start() + self.assertTrue(result.access(arg0)) + return result + + def test_access(self): + a = self.setup_accessor('.treeinfo') self.assertFalse(a.access('no_such_file')) self.assertEqual(a.lastError, 404) a.finish() - def test_file(self): - a = xcp.accessor.createAccessor("file://tests/data/repo/", True) - a.start() - self.assertTrue(a.access('.treeinfo')) - self.assertFalse(a.access('no_such_file')) - self.assertEqual(a.lastError, 404) + def test_file_binfile(self): + binary_file_in_repo = "boot/isolinux/mboot.c32" + a = self.setup_accessor(binary_file_in_repo) + inf = a.openAddress(binary_file_in_repo) + with NamedTemporaryFile("wb") as outf: + while True: + data = inf.read() + if not data: # EOF + break + outf.write(data) + outf.flush() + hasher = hashlib.md5() + with open(outf.name, "rb") as bincontents: + while True: + data = bincontents.read() + if not data: # EOF + break + hasher.update(data) + csum = hasher.hexdigest() + self.assertEqual(csum, "eab52cebc3723863432dc672360f6dac") a.finish() diff --git a/tests/test_cpio.py b/tests/test_cpio.py index c3543c89..8592baf2 100644 --- a/tests/test_cpio.py +++ b/tests/test_cpio.py @@ -14,11 +14,11 @@ def writeRandomFile(fn, size, start=b'', add=b'a'): with open(fn, 'wb') as f: m = md5() m.update(start) - assert(len(add) != 0) + assert add while size > 0: d = m.digest() if size < len(d): - d=d[:size] + d = d[:size] f.write(d) size -= len(d) m.update(add) @@ -44,8 +44,10 @@ def setUp(self): os.utime('archive/data', (0, 0)) os.utime('archive', (0, 0)) - check_call( - "find archive | cpio --reproducible -o -H newc > archive.cpio") + try: + check_call("find archive | cpio --reproducible -o -H newc > archive.cpio") + except: + raise unittest.SkipTest("cpio tool not available") check_call("gzip -c < archive.cpio > archive.cpio.gz") check_call("bzip2 -c < archive.cpio > archive.cpio.bz2") try: diff --git a/tests/test_dom0.py b/tests/test_dom0.py index bf198341..440bb109 100644 --- a/tests/test_dom0.py +++ b/tests/test_dom0.py @@ -30,7 +30,7 @@ def mock_version(open_mock, version): (2*1024, 4*1024, 8*1024), # Above max ] - with patch("__builtin__.open") as open_mock: + with patch("xcp.dom0.open") as open_mock: for host_gib, dom0_mib, _ in test_values: mock_version(open_mock, '2.8.0') expected = dom0_mib * 1024; @@ -39,7 +39,7 @@ def mock_version(open_mock, version): open_mock.assert_called_with("/etc/xensource-inventory") - with patch("__builtin__.open") as open_mock: + with patch("xcp.dom0.open") as open_mock: for host_gib, _, dom0_mib in test_values: mock_version(open_mock, '2.9.0') expected = dom0_mib * 1024; diff --git a/tests/test_ifrename_dynamic.py b/tests/test_ifrename_dynamic.py index 1cc95e39..0948d254 100644 --- a/tests/test_ifrename_dynamic.py +++ b/tests/test_ifrename_dynamic.py @@ -125,10 +125,10 @@ def test_pci_matching_invert(self): MACPCI("c8:cb:b8:d3:0c:cf", "0000:04:00.0", kname="eth1", ppn="", label="")]) - self.assertEqual(dr.rules,[ + self.assertEqual(set(dr.rules), set([ MACPCI("c8:cb:b8:d3:0c:ce", "0000:04:00.0", tname="eth1"), MACPCI("c8:cb:b8:d3:0c:cf", "0000:04:00.0", tname="eth0") - ]) + ])) def test_pci_missing(self): diff --git a/tests/test_ifrename_logic.py b/tests/test_ifrename_logic.py index 3bb7a911..a2715334 100644 --- a/tests/test_ifrename_logic.py +++ b/tests/test_ifrename_logic.py @@ -518,6 +518,7 @@ def test_ibft_nic_to_ibft(self): class TestInputSanitisation(unittest.TestCase): + # pylint: disable=no-member def setUp(self): """ diff --git a/tests/test_ifrename_static.py b/tests/test_ifrename_static.py index 9b11e380..674decfe 100644 --- a/tests/test_ifrename_static.py +++ b/tests/test_ifrename_static.py @@ -375,10 +375,10 @@ def test_pci_matching(self): sr.generate(self.state) - self.assertEqual(sr.rules,[ - MACPCI("c8:cb:b8:d3:0c:cf", "0000:04:00.0", tname="eth1"), - MACPCI("c8:cb:b8:d3:0c:ce", "0000:04:00.0", tname="eth0") - ]) + self.assertEqual(set(sr.rules), set([ + MACPCI("c8:cb:b8:d3:0c:cf", "0000:04:00.0", tname="eth1"), + MACPCI("c8:cb:b8:d3:0c:ce", "0000:04:00.0", tname="eth0") + ])) def test_pci_matching_invert(self): @@ -389,10 +389,10 @@ def test_pci_matching_invert(self): sr.generate(self.state) - self.assertEqual(sr.rules,[ - MACPCI("c8:cb:b8:d3:0c:ce", "0000:04:00.0", tname="eth1"), - MACPCI("c8:cb:b8:d3:0c:cf", "0000:04:00.0", tname="eth0") - ]) + self.assertEqual(set(sr.rules), set([ + MACPCI("c8:cb:b8:d3:0c:ce", "0000:04:00.0", tname="eth1"), + MACPCI("c8:cb:b8:d3:0c:cf", "0000:04:00.0", tname="eth0") + ])) def test_pci_matching_mixed(self): @@ -403,10 +403,10 @@ def test_pci_matching_mixed(self): sr.generate(self.state) - self.assertEqual(sr.rules,[ - MACPCI("c8:cb:b8:d3:0c:cf", "0000:04:00.0", tname="eth0"), - MACPCI("c8:cb:b8:d3:0c:ce", "0000:04:00.0", tname="eth1") - ]) + self.assertEqual(set(sr.rules), set([ + MACPCI("c8:cb:b8:d3:0c:cf", "0000:04:00.0", tname="eth0"), + MACPCI("c8:cb:b8:d3:0c:ce", "0000:04:00.0", tname="eth1") + ])) def test_pci_missing(self): diff --git a/tests/test_repository.py b/tests/test_repository.py index 833627d0..4768740d 100644 --- a/tests/test_repository.py +++ b/tests/test_repository.py @@ -6,7 +6,7 @@ class TestRepository(unittest.TestCase): def test_http(self): - raise unittest.SkipTest("comment out if you really mean it") + #raise unittest.SkipTest("comment out if you really mean it") a = xcp.accessor.createAccessor("https://updates.xcp-ng.org/netinstall/8.2.1", True) repo_ver = repository.BaseRepository.getRepoVer(a) self.assertEqual(repo_ver, Version([3, 2, 1])) diff --git a/tox.ini b/tox.ini new file mode 100644 index 00000000..65bb7fd1 --- /dev/null +++ b/tox.ini @@ -0,0 +1,96 @@ +[tox] +skipsdist = true +isolated_build = true +envlist = py27-test, py36-test, py37-test, py38-test, py39-test, py310-test, + py311-mypy, py311-cov, py312-test + +# TODO later: Enable the venv_update extension so that changes to requirements-dev.txt +# are automatically detected. +tox_pip_extensions_ext_venv_update = true + +[testenv] +description = + Run in a {basepython} virtualenv: + test: pytest + cov: generate coverage html reports + fox: generate coverage html reports and open them in firefox + mypy: mypy static analyis + covcp: copy the generated .converage and coverage.xml to the UPLOAD_DIR dir for upload +passenv = PYTEST_ADDOPTS + PYTEST_XDIST_WORKER_COUNT + covcp: UPLOAD_DIR + covcp: HOME + mypy: TERM + mypy: MYPY_FORCE_COLOR + mypy: MYPY_FORCE_TERMINAL_WIDTH + fox: DISPLAY + fox: XAUTHORITY + fox: DBUS_SESSION_BUS_ADDRESS +setenv = PYTHONWARNINGS=ignore:DEPRECATION +deps = + {cov,covcp,fox,test}: -r requirements-dev.txt + diff: diff-cov-lint + mypy: lxml + mypy: mypy + mypy: types-mock + mypy: types-simplejson + mypy: types-six +allowlist_externals = + {cov,covcp,fox}: cp + diff: sh + mypy: cat + fox: firefox +commands = + {cov,covcp,fox,test}: pytest --cov + {cov,covcp,fox}: {[cov]commands} + covcp: cp -av {envlogdir}/coverage.xml {env:UPLOAD_DIR:.} + mypy: mypy --txt-report . + mypy: cat index.txt + fox: {[fox]commands} + diff: {[diff]commands} + +[cov] +commands = + coverage xml -o {envlogdir}/coverage.xml + coverage html -d {envlogdir}/htmlcov + coverage html -d {envlogdir}/htmlcov-tests --include="tests/*" + diff-cover --compare-branch=origin/master \ + --html-report {envlogdir}/coverage-diff.html \ + {envlogdir}/coverage.xml + pylint --output {envlogdir}/pylint.txt --exit-zero \ + --msg-template='\{path\}:\{line\}: [\{msg_id\}(\{symbol\}), \{obj\}] \{msg\}' \ + xcp/ tests/ + diff-quality --compare-branch=origin/master --violations=pylint \ + --html-report {envlogdir}/pylint-diff.html {envlogdir}/pylint.txt + cp -av .coverage {envlogdir}/.coverage + cp -av {envlogdir}/coverage.xml . + +[diff] +commands = + pylint --enable-all-extensions --disable=C,R,W0149,W0160,W0707 --exit-zero \ + --output {envlogdir}/pylint-warnings.txt xcp/ tests/ + sh -c "diff-cov-lint origin/master . \ + --lint_report={envlogdir}/pylint-warnings.txt \ + > {envlogdir}/pylint-warnings-on-changed-lines.txt" + sh -c 'if grep / {envlogdir}/pylint-warnings-on-changed-lines.txt;then \ + cat {envlogdir}/pylint-warnings-on-changed-lines.txt;exit 5;fi' + +[fox] +commands = firefox {envlogdir}/htmlcov/index.html \ + {envlogdir}/htmlcov-tests/index.html \ + {envlogdir}/coverage-diff.html + +# Map the github python versions to environments to testenvs to run on them: +# See https://github.com/ymyzk/tox-gh-actions for details: +# https://github.com/ymyzk/tox-gh-actions#tox-gh-actions-configuration +# The benefit of using tox is that all versions can be run locally and +# the local venvs will be the same as the venvs created by tox on the GitHub runners: +[gh-actions] +python = + 2.7: py27-test + 3.6: py36-test + 3.7: py37-test + 3.8: py38-test + 3.9: py39-test + 3.10: py310-mypy + 3.11: py311-cov-diff \ No newline at end of file diff --git a/xcp/accessor.py b/xcp/accessor.py index 6d057927..e7d8cc59 100644 --- a/xcp/accessor.py +++ b/xcp/accessor.py @@ -25,17 +25,13 @@ """accessor - provide common interface to access methods""" -# pylint: disable=wrong-import-position,wrong-import-order -from future import standard_library -standard_library.install_aliases() - import ftplib import os import tempfile -import urllib.request # pylint: disable=import-error -import urllib.error # pylint: disable=import-error -import urllib.parse # pylint: disable=import-error import errno +from abc import abstractmethod + +from six.moves import urllib # pyright: ignore import xcp.mount as mount import xcp.logger as logger @@ -58,11 +54,11 @@ def __init__(self, ro): self.read_only = ro self.lastError = 0 - def access(self, name): + def access(self, address): """ Return boolean determining where 'name' is an accessible object in the target. """ try: - f = self.openAddress(name) + f = self.openAddress(address) if not f: return False f.close() @@ -71,9 +67,9 @@ def access(self, name): return True - def openAddress(self, address): - """should be overloaded""" - pass + @abstractmethod + def openAddress(self, address, mode="", **kwargs): + """must be overloaded, accept mode and kwargs and return a file handle""" def canEject(self): return False @@ -99,9 +95,12 @@ def __init__(self, location, ro): super(FilesystemAccessor, self).__init__(ro) self.location = location - def openAddress(self, address): - try: - filehandle = open(os.path.join(self.location, address), 'r') + def openAddress(self, address, mode="rb", **kwargs): + # To be safe against mis-use like `openAccess(file, mode="r")` for text mode + # (note: the mode parameter is new), enforce "rb" unless encoding was given: + mode = mode if "encoding" in kwargs else "rb" + try: # pylint: disable-next=unspecified-encoding,consider-using-with + filehandle = open(os.path.join(self.location, address), mode, **kwargs) except OSError as e: if e.errno == errno.EIO: self.lastError = 5 @@ -165,9 +164,13 @@ def finish(self): os.rmdir(self.location) self.location = None - def writeFile(self, in_fh, out_name): + def writeFile(self, in_fh, out_name, mode="wb", **kwargs): logger.info("Copying to %s" % os.path.join(self.location, out_name)) - out_fh = open(os.path.join(self.location, out_name), 'w') + # To be safe against mis-use like `writeFile(file, mode="w")` for text mode + # (note: the mode parameter is new), enforce "wb" unless encoding was given: + mode = mode if "encoding" in kwargs else "wb" + # pylint: disable-next=unspecified-encoding,consider-using-with + out_fh = open(os.path.join(self.location, out_name), mode, **kwargs) return self._writeFile(in_fh, out_fh) def __del__(self): @@ -220,9 +223,12 @@ def __init__(self, baseAddress, ro): super(FileAccessor, self).__init__(ro) self.baseAddress = baseAddress - def openAddress(self, address): - try: - file = open(os.path.join(self.baseAddress, address)) + def openAddress(self, address, mode="rb", **kwargs): + # To be safe against mis-use like `openAccess(file, mode="r")` for text mode + # (note: the mode parameter is new), enforce "rb" unless encoding was given: + mode = mode if "encoding" in kwargs else "rb" + try: # pylint: disable-next=unspecified-encoding,consider-using-with + file = open(os.path.join(self.baseAddress, address), mode, **kwargs) except IOError as e: if e.errno == errno.EIO: self.lastError = 5 @@ -240,9 +246,13 @@ def openAddress(self, address): return False return file - def writeFile(self, in_fh, out_name): + def writeFile(self, in_fh, out_name, mode="wb", **kwargs): + # To be safe against mis-use like `writeFile(file, mode="w")` for text mode + # (note: the mode parameter is new), enforce "wb" unless encoding was given: + mode = mode if "encoding" in kwargs else "wb" logger.info("Copying to %s" % os.path.join(self.baseAddress, out_name)) - out_fh = open(os.path.join(self.baseAddress, out_name), 'w') + # pylint: disable-next=unspecified-encoding,consider-using-with + out_fh = open(os.path.join(self.baseAddress, out_name), mode, **kwargs) return self._writeFile(in_fh, out_fh) def __repr__(self): @@ -267,6 +277,11 @@ def __init__(self, baseAddress, ro): self.ftp = None self.baseAddress = rebuild_url(self.url_parts) + def _parse_ftp_url(self, debug_message, ftp_url): + logger.debug(debug_message + " " + ftp_url) + self._cleanup() + return urllib.parse.unquote(ftp_url) + def _cleanup(self): if self.cleanup: # clean up after RETR @@ -276,10 +291,8 @@ def _cleanup(self): def start(self): if self.start_count == 0: self.ftp = ftplib.FTP() - #self.ftp.set_debuglevel(1) - port = ftplib.FTP_PORT - if self.url_parts.port: - port = self.url_parts.port + port = self.url_parts.port or ftplib.FTP_PORT + assert self.url_parts.hostname self.ftp.connect(self.url_parts.hostname, port) username = self.url_parts.username password = self.url_parts.password @@ -305,12 +318,9 @@ def finish(self): self.cleanup = False self.ftp = None - def access(self, path): + def access(self, address): try: - logger.debug("Testing "+path) - self._cleanup() - url = urllib.parse.unquote(path) - + url = self._parse_ftp_url("Testing", address) if self.ftp.size(url) is not None: return True lst = self.ftp.nlst(os.path.dirname(url)) @@ -331,13 +341,10 @@ def access(self, path): self.lastError = 500 return False - def openAddress(self, address): - logger.debug("Opening "+address) - self._cleanup() - url = urllib.parse.unquote(address) - + def openAddress(self, address, mode="rb", **kwargs): + url = self._parse_ftp_url("Opening", address) self.ftp.voidcmd('TYPE I') - s = self.ftp.transfercmd('RETR ' + url).makefile('rb') + s = self.ftp.transfercmd('RETR ' + url).makefile(mode, **kwargs) self.cleanup = True return s @@ -373,8 +380,10 @@ def __init__(self, baseAddress, ro): self.baseAddress = rebuild_url(self.url_parts) - def openAddress(self, address): + def openAddress(self, address, mode="", **kwargs): + """Open an HTTP/S URL. Note urllib must return binary as encoding may be gzip""" try: + # pylint: disable-next=consider-using-with urlFile = urllib.request.urlopen(os.path.join(self.baseAddress, address)) except urllib.error.HTTPError as e: self.lastError = e.code diff --git a/xcp/bootloader.py b/xcp/bootloader.py index a1d19709..81a61bd4 100644 --- a/xcp/bootloader.py +++ b/xcp/bootloader.py @@ -336,19 +336,19 @@ def create_label(title): try: for line in fh: l = line.strip() - menu_match = re.match("menuentry ['\"]([^']*)['\"](.*){", l) + menu_match = re.match(r"menuentry ['\"]([^']*)['\"](.*){", l) # Only parse unindented default and timeout lines to prevent # changing these lines in if statements. if l.startswith('set default=') and l == line.rstrip(): default = l.split('=')[1] - match = re.match("['\"](.*)['\"]$", default) + match = re.match(r"['\"](.*)['\"]$", default) if match: default = match.group(1) elif l.startswith('set timeout=') and l == line.rstrip(): timeout = int(l.split('=')[1]) * 10 elif l.startswith('serial'): - match = re.match("serial --unit=(\d+) --speed=(\d+)", l) + match = re.match(r"serial --unit=(\d+) --speed=(\d+)", l) if match: serial = { 'port': int(match.group(1)), diff --git a/xcp/cmd.py b/xcp/cmd.py index bbd94656..aa8a847e 100644 --- a/xcp/cmd.py +++ b/xcp/cmd.py @@ -28,11 +28,11 @@ import xcp.logger as logger def runCmd(command, with_stdout = False, with_stderr = False, inputtext = None): - cmd = subprocess.Popen(command, bufsize = 1, - stdin = (inputtext and subprocess.PIPE or None), - stdout = subprocess.PIPE, - stderr = subprocess.PIPE, - shell = isinstance(command, str)) + cmd = subprocess.Popen(command, bufsize=1, + stdin=(inputtext and subprocess.PIPE or None), + stdout=subprocess.PIPE, + stderr=subprocess.PIPE, + shell=isinstance(command, str)) (out, err) = cmd.communicate(inputtext) rv = cmd.returncode diff --git a/xcp/cpiofile.py b/xcp/cpiofile.py index 7b2623fa..24ea559d 100755 --- a/xcp/cpiofile.py +++ b/xcp/cpiofile.py @@ -64,7 +64,7 @@ try: import grp as GRP, pwd as PWD except ImportError: - GRP = PWD = None + GRP = PWD = None # type: ignore # from cpiofile import * __all__ = ["CpioFile", "CpioInfo", "is_cpiofile", "CpioError"] @@ -310,7 +310,7 @@ def _init_write_gz(self): self.__write(b"\037\213\010\010%s\002\377" % timestamp) if self.name.endswith(".gz"): self.name = self.name[:-3] - self.__write(self.name + NUL) + self.__write(six.ensure_binary(self.name) + NUL) def write(self, s): """Write string s to the stream. @@ -951,7 +951,7 @@ def __init__(self, name=None, mode="r", fileobj=None): self.mode = {"r": "rb", "a": "r+b", "w": "wb"}[mode] if not fileobj: - fileobj = file(name, self.mode) + fileobj = bltn_open(name, self.mode) self._extfileobj = False else: if name is None and hasattr(fileobj, "name"): @@ -1109,7 +1109,7 @@ def gzopen(cls, name, mode="r", fileobj=None, compresslevel=9): raise CompressionError("gzip module is not available") if fileobj is None: - fileobj = file(name, mode + "b") + fileobj = bltn_open(name, mode + "b") try: t = cls.cpioopen(name, mode, gzip.GzipFile(name, mode, compresslevel, fileobj)) @@ -1354,7 +1354,7 @@ def add(self, name, arcname=None, recursive=True): # Append the cpio header and data to the archive. if cpioinfo.isreg(): - f = file(name, "rb") + f = bltn_open(name, "rb") self.addfile(cpioinfo, f) f.close() @@ -1420,7 +1420,7 @@ def extractall(self, path=".", members=None): # Extract directory with a safe mode, so that # all files below can be extracted as well. try: - os.makedirs(os.path.join(path, cpioinfo.name), 0o777) + os.makedirs(os.path.join(path, six.ensure_text(cpioinfo.name)), 0o777) except EnvironmentError: pass directories.append(cpioinfo) @@ -1428,12 +1428,12 @@ def extractall(self, path=".", members=None): self.extract(cpioinfo, path) # Reverse sort directories. - directories.sort(lambda a, b: cmp(a.name, b.name)) + directories.sort(key=lambda x: x.name) directories.reverse() # Set correct owner, mtime and filemode on directories. for cpioinfo in directories: - path = os.path.join(path, cpioinfo.name) + path = os.path.join(path, six.ensure_text(cpioinfo.name)) try: self.chown(cpioinfo, path) self.utime(cpioinfo, path) @@ -1462,7 +1462,7 @@ def extract(self, member, path=""): cpioinfo._link_path = path try: - self._extract_member(cpioinfo, os.path.join(path, cpioinfo.name)) + self._extract_member(cpioinfo, os.path.join(path, six.ensure_text(cpioinfo.name))) except EnvironmentError as e: if self.errorlevel > 0: raise @@ -1594,7 +1594,7 @@ def makefile(self, cpioinfo, targetpath): if extractinfo: source = self.extractfile(extractinfo) - target = file(targetpath, "wb") + target = bltn_open(targetpath, "wb") copyfileobj(source, target) source.close() target.close() @@ -1926,5 +1926,5 @@ def is_cpiofile(name): except CpioError: return False -def cpioOpen(*al, **ad): - return CpioFile.open(*al, **ad) +bltn_open = open +open = CpioFile.open # pylint: disable=redefined-builtin diff --git a/xcp/dom0.py b/xcp/dom0.py index 086a683b..b8a46c3a 100644 --- a/xcp/dom0.py +++ b/xcp/dom0.py @@ -96,7 +96,7 @@ def default_memory(host_mem_kib): return default_memory_for_version(host_mem_kib, platform_version) -_size_and_unit_re = re.compile("^(-?\d+)([bkmg]?)$", re.IGNORECASE) +_size_and_unit_re = re.compile(r"^(-?\d+)([bkmg]?)$", re.IGNORECASE) def _parse_size_and_unit(s): m = _size_and_unit_re.match(s) diff --git a/xcp/net/ifrename/dynamic.py b/xcp/net/ifrename/dynamic.py index 33040ffd..cc5a211b 100644 --- a/xcp/net/ifrename/dynamic.py +++ b/xcp/net/ifrename/dynamic.py @@ -39,7 +39,7 @@ import json except ImportError: try: - import simplejson as json + import simplejson as json # type: ignore # mypy sees the import above # The installer has no json. In the meantime, there is a workaround except ImportError: pass diff --git a/xcp/net/ifrename/logic.py b/xcp/net/ifrename/logic.py index e2a3484b..41e74c02 100644 --- a/xcp/net/ifrename/logic.py +++ b/xcp/net/ifrename/logic.py @@ -52,9 +52,9 @@ from xcp.logger import LOG from xcp.net.ifrename.macpci import MACPCI -VALID_CUR_STATE_KNAME = re.compile("^(?:eth[\d]+|side-[\d]+-eth[\d]+)$") -VALID_ETH_NAME = re.compile("^eth([\d])+$") -VALID_IBFT_NAME = re.compile("^ibft([\d])+$") +VALID_CUR_STATE_KNAME = re.compile(r"^(?:eth[\d]+|side-[\d]+-eth[\d]+)$") +VALID_ETH_NAME = re.compile(r"^eth([\d])+$") +VALID_IBFT_NAME = re.compile(r"^ibft([\d])+$") # util needs to import VALID_ETH_NAME from xcp.net.ifrename import util @@ -289,9 +289,10 @@ def rename_logic( static_rules, # Check that the function still has the same number of nics if len(lastnics) != len(newnics): - LOG.warn("multi-nic function %s had %d nics but now has %d. " - "Defering all until later for renaming" - % (fn, len(lastnics), len(newnics))) + LOG.warning( + "multi-nic function %s had %d nics but now has %d. " + "Defering all until later for renaming", + fn, len(lastnics), len(newnics)) continue # Check that all nics are still pending a rename diff --git a/xcp/net/ifrename/static.py b/xcp/net/ifrename/static.py index c1b9b100..bf503d54 100644 --- a/xcp/net/ifrename/static.py +++ b/xcp/net/ifrename/static.py @@ -89,7 +89,7 @@ class StaticRules(object): methods = ["mac", "pci", "ppn", "label", "guess"] validators = { "mac": VALID_MAC, "pci": VALID_PCI, - "ppn": re.compile("^(?:em\d+|p(?:ci)?\d+p\d+)$") + "ppn": re.compile(r"^(?:em\d+|p(?:ci)?\d+p\d+)$") } def __init__(self, path=None, fd=None): diff --git a/xcp/net/mac.py b/xcp/net/mac.py index 47e586c0..56ba4b7b 100644 --- a/xcp/net/mac.py +++ b/xcp/net/mac.py @@ -31,11 +31,13 @@ __author__ = "Andrew Cooper" import re +import six VALID_COLON_MAC = re.compile(r"^([\da-fA-F]{1,2}:){5}[\da-fA-F]{1,2}$") VALID_DASH_MAC = re.compile(r"^([\da-fA-F]{1,2}-){5}[\da-fA-F]{1,2}$") VALID_DOTQUAD_MAC = re.compile(r"^([\da-fA-F]{1,4}\.){2}[\da-fA-F]{1,4}$") +@six.python_2_unicode_compatible class MAC(object): """ Mac address object for manipulation and comparison @@ -59,27 +61,25 @@ def __init__(self, addr): self.octets = [] self.integer = -1 - if isinstance(addr, (str, unicode)): + if not isinstance(addr, six.string_types): + raise TypeError("String expected") - res = VALID_COLON_MAC.match(addr) - if res: - self._set_from_str_octets(addr.split(":")) - return + res = VALID_COLON_MAC.match(addr) + if res: + self._set_from_str_octets(addr.split(":")) + return - res = VALID_DASH_MAC.match(addr) - if res: - self._set_from_str_octets(addr.split("-")) - return + res = VALID_DASH_MAC.match(addr) + if res: + self._set_from_str_octets(addr.split("-")) + return - res = VALID_DOTQUAD_MAC.match(addr) - if res: - self._set_from_str_quads(addr.split(".")) - return + res = VALID_DOTQUAD_MAC.match(addr) + if res: + self._set_from_str_quads(addr.split(".")) + return - raise ValueError("Unrecognised MAC address '%s'" % addr) - - else: - raise TypeError("String expected") + raise ValueError("Unrecognised MAC address '%s'" % addr) def _set_from_str_octets(self, octets): @@ -122,9 +122,6 @@ def is_local(self): def __str__(self): - return unicode(self).encode('utf-8') - - def __unicode__(self): return ':'.join([ "%0.2x" % x for x in self.octets]) def __repr__(self): diff --git a/xcp/pci.py b/xcp/pci.py index 1c8e081d..ffc52a33 100644 --- a/xcp/pci.py +++ b/xcp/pci.py @@ -24,11 +24,12 @@ import os.path import subprocess import re +import six _SBDF = (r"(?:(?P [\da-dA-F]{4}):)?" # Segment (optional) - " (?P [\da-fA-F]{2}):" # Bus - " (?P [\da-fA-F]{2})\." # Device - " (?P[\da-fA-F])" # Function + r" (?P [\da-fA-F]{2}):" # Bus + r" (?P [\da-fA-F]{2})\." # Device + r" (?P[\da-fA-F])" # Function ) # Don't change the meaning of VALID_SBDF as some parties may be using it @@ -36,7 +37,7 @@ VALID_SBDFI = re.compile( r"^(?P%s)" - " (?:[[](?P[\d]{1,2})[]])?$" # Index (optional) + r" (?:[[](?P[\d]{1,2})[]])?$" # Index (optional) % _SBDF , re.X) @@ -66,48 +67,46 @@ def __init__(self, addr): self.function = -1 self.index = -1 - if isinstance(addr, (str, unicode)): - - res = VALID_SBDFI.match(addr) - if res: - groups = res.groupdict() + if not isinstance(addr, six.string_types): + raise TypeError("String expected") - if "segment" in groups and groups["segment"] is not None: - self.segment = int(groups["segment"], 16) - else: - self.segment = 0 + res = VALID_SBDFI.match(addr) + if res: + groups = res.groupdict() - self.bus = int(groups["bus"], 16) - if not ( 0 <= self.bus < 2**8 ): - raise ValueError("Bus '%d' out of range 0 <= bus < 256" - % (self.bus,)) + if "segment" in groups and groups["segment"] is not None: + self.segment = int(groups["segment"], 16) + else: + self.segment = 0 - self.device = int(groups["device"], 16) - if not ( 0 <= self.device < 2**5): - raise ValueError("Device '%d' out of range 0 <= device < 32" - % (self.device,)) + self.bus = int(groups["bus"], 16) + if not ( 0 <= self.bus < 2**8 ): + raise ValueError("Bus '%d' out of range 0 <= bus < 256" + % (self.bus,)) - self.function = int(groups["function"], 16) - if not ( 0 <= self.function < 2**3): - raise ValueError("Function '%d' out of range 0 <= device " - "< 8" % (self.function,)) + self.device = int(groups["device"], 16) + if not ( 0 <= self.device < 2**5): + raise ValueError("Device '%d' out of range 0 <= device < 32" + % (self.device,)) - if "index" in groups and groups["index"] is not None: - self.index = int(groups["index"]) - else: - self.index = 0 + self.function = int(groups["function"], 16) + if not ( 0 <= self.function < 2**3): + raise ValueError("Function '%d' out of range 0 <= device " + "< 8" % (self.function,)) - self.integer = (int(self.segment << 16 | - self.bus << 8 | - self.device << 3 | - self.function) << 8 | - self.index) - return + if "index" in groups and groups["index"] is not None: + self.index = int(groups["index"]) + else: + self.index = 0 - raise ValueError("Unrecognised PCI address '%s'" % addr) + self.integer = (int(self.segment << 16 | + self.bus << 8 | + self.device << 3 | + self.function) << 8 | + self.index) + return - else: - raise TypeError("String expected") + raise ValueError("Unrecognised PCI address '%s'" % addr) def __str__(self): diff --git a/xcp/repository.py b/xcp/repository.py index ca284647..b10aa092 100644 --- a/xcp/repository.py +++ b/xcp/repository.py @@ -23,10 +23,12 @@ # (INCLUDING NEGLIGENCE OR OTHERWISE) ARISING IN ANY WAY OUT OF THE USE OF THIS # SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF SUCH DAMAGE. -import md5 +from hashlib import md5 +import io import os.path import xml.dom.minidom -import ConfigParser +import configparser +import sys import six @@ -179,10 +181,17 @@ def _getVersion(cls, access, category): access.start() try: - treeinfofp = access.openAddress(cls.TREEINFO_FILENAME) - treeinfo = ConfigParser.SafeConfigParser() - treeinfo.readfp(treeinfofp) - treeinfofp.close() + rawtreeinfofp = access.openAddress(cls.TREEINFO_FILENAME) + if sys.version_info < (3, 0) or isinstance(rawtreeinfofp, io.TextIOBase): + # e.g. with FileAccessor + treeinfofp = rawtreeinfofp + else: + # e.g. with HTTPAccessor + treeinfofp = io.TextIOWrapper(rawtreeinfofp, encoding='utf-8') + treeinfo = configparser.ConfigParser() + treeinfo.read_file(treeinfofp) + treeinfofp = None + rawtreeinfofp.close() if treeinfo.has_section('system-v1'): ver_str = treeinfo.get('system-v1', category_map[category]) else: @@ -246,7 +255,7 @@ def findRepositories(cls, access): def __init__(self, access, base, is_group = False): BaseRepository.__init__(self, access, base) self.is_group = is_group - self._md5 = md5.new() + self._md5 = md5() self.requires = [] self.packages = [] diff --git a/xcp/xmlunwrap.py b/xcp/xmlunwrap.py index 1487afab..0523bd12 100644 --- a/xcp/xmlunwrap.py +++ b/xcp/xmlunwrap.py @@ -34,7 +34,9 @@ def getText(nodelist): for node in nodelist.childNodes: if node.nodeType == node.TEXT_NODE: rc = rc + node.data - return rc.encode().strip() + if not isinstance(rc, str): # Python 2 only, otherwise it would return unicode + rc = rc.encode() + return rc.strip() def getElementsByTagName(el, tags, mandatory = False): matching = [] @@ -47,7 +49,9 @@ def getElementsByTagName(el, tags, mandatory = False): def getStrAttribute(el, attrs, default = '', mandatory = False): matching = [] for attr in attrs: - val = el.getAttribute(attr).encode() + val = el.getAttribute(attr) + if not isinstance(val, str): # Python 2 only, otherwise it would return unicode + val = val.encode() if val != '': matching.append(val) if len(matching) == 0: