public inbox for pve-devel@lists.proxmox.com
 help / color / mirror / Atom feed
* [pve-devel] [PATCH v2 pve-esxi-import-tools 0/7] Improve listvms.py
@ 2024-03-22 18:06 Max Carrara
  2024-03-22 18:06 ` [pve-devel] [PATCH v2 pve-esxi-import-tools 1/7] listvms: remove unused import and variable Max Carrara
                   ` (7 more replies)
  0 siblings, 8 replies; 9+ messages in thread
From: Max Carrara @ 2024-03-22 18:06 UTC (permalink / raw)
  To: pve-devel

Improve listvms.py - Version 2
==============================

Notable Changes Since v1
------------------------

* mypy [0] is now a build dependency and runs automatically on
  `make install` (and thus also on `make deb` etc.)
* JSON output is now directly streamed to stdout via `json.dump()`
  instead of creating and printing a string

For a detailed list of changes, please see the comments in the
invididual patches.

Older Versions
--------------

v1: https://lists.proxmox.com/pipermail/pve-devel/2024-March/062258.html

References
----------

[0]: https://www.mypy-lang.org/

Summary of Changes
------------------

Max Carrara (7):
  listvms: remove unused import and variable
  listvms: reorder imports
  listvms: improve typing and add dataclasses to represent dicts
  listvms: add arg parser, context manager for connections, fetch helper
  listvms: dump json directly to stdout
  listvms: run formatter
  use mypy for automatic type checks in Python

 Makefile       |  13 ++-
 debian/control |   1 +
 listvms.py     | 300 +++++++++++++++++++++++++++++++++++++------------
 mypy.ini       |   8 ++
 4 files changed, 247 insertions(+), 75 deletions(-)
 create mode 100644 mypy.ini

-- 
2.39.2





^ permalink raw reply	[flat|nested] 9+ messages in thread

* [pve-devel] [PATCH v2 pve-esxi-import-tools 1/7] listvms: remove unused import and variable
  2024-03-22 18:06 [pve-devel] [PATCH v2 pve-esxi-import-tools 0/7] Improve listvms.py Max Carrara
@ 2024-03-22 18:06 ` Max Carrara
  2024-03-22 18:06 ` [pve-devel] [PATCH v2 pve-esxi-import-tools 2/7] listvms: reorder imports Max Carrara
                   ` (6 subsequent siblings)
  7 siblings, 0 replies; 9+ messages in thread
From: Max Carrara @ 2024-03-22 18:06 UTC (permalink / raw)
  To: pve-devel

Signed-off-by: Max Carrara <m.carrara@proxmox.com>
---
Changes v1 --> v2:
  * none

 listvms.py | 2 --
 1 file changed, 2 deletions(-)

diff --git a/listvms.py b/listvms.py
index 0a1b830..d52d184 100755
--- a/listvms.py
+++ b/listvms.py
@@ -2,7 +2,6 @@
 
 from typing import List, Dict, Optional
 import json
-import os
 import ssl
 import sys
 from pyVim.connect import SmartConnect, Disconnect
@@ -103,7 +102,6 @@ def main():
         sys.exit(1)
 
     try:
-        datacenters = get_all_datacenters(si)
         vms = list_vms(si)
         data = {}
         for vm in vms:
-- 
2.39.2





^ permalink raw reply	[flat|nested] 9+ messages in thread

* [pve-devel] [PATCH v2 pve-esxi-import-tools 2/7] listvms: reorder imports
  2024-03-22 18:06 [pve-devel] [PATCH v2 pve-esxi-import-tools 0/7] Improve listvms.py Max Carrara
  2024-03-22 18:06 ` [pve-devel] [PATCH v2 pve-esxi-import-tools 1/7] listvms: remove unused import and variable Max Carrara
@ 2024-03-22 18:06 ` Max Carrara
  2024-03-22 18:06 ` [pve-devel] [PATCH v2 pve-esxi-import-tools 3/7] listvms: improve typing and add dataclasses to represent dicts Max Carrara
                   ` (5 subsequent siblings)
  7 siblings, 0 replies; 9+ messages in thread
From: Max Carrara @ 2024-03-22 18:06 UTC (permalink / raw)
  To: pve-devel

Signed-off-by: Max Carrara <m.carrara@proxmox.com>
---
Changes v1 --> v2:
  * remove unused import of `Tuple` type
    (was accidentally added during spurious rebasing)

 listvms.py | 4 +++-
 1 file changed, 3 insertions(+), 1 deletion(-)

diff --git a/listvms.py b/listvms.py
index d52d184..0b64b0b 100755
--- a/listvms.py
+++ b/listvms.py
@@ -1,9 +1,11 @@
 #!/usr/bin/python3
 
-from typing import List, Dict, Optional
 import json
 import ssl
 import sys
+
+from typing import List, Dict, Optional
+
 from pyVim.connect import SmartConnect, Disconnect
 from pyVmomi import vim
 
-- 
2.39.2





^ permalink raw reply	[flat|nested] 9+ messages in thread

* [pve-devel] [PATCH v2 pve-esxi-import-tools 3/7] listvms: improve typing and add dataclasses to represent dicts
  2024-03-22 18:06 [pve-devel] [PATCH v2 pve-esxi-import-tools 0/7] Improve listvms.py Max Carrara
  2024-03-22 18:06 ` [pve-devel] [PATCH v2 pve-esxi-import-tools 1/7] listvms: remove unused import and variable Max Carrara
  2024-03-22 18:06 ` [pve-devel] [PATCH v2 pve-esxi-import-tools 2/7] listvms: reorder imports Max Carrara
@ 2024-03-22 18:06 ` Max Carrara
  2024-03-22 18:06 ` [pve-devel] [PATCH v2 pve-esxi-import-tools 4/7] listvms: add arg parser, context manager for connections, fetch helper Max Carrara
                   ` (4 subsequent siblings)
  7 siblings, 0 replies; 9+ messages in thread
From: Max Carrara @ 2024-03-22 18:06 UTC (permalink / raw)
  To: pve-devel

This commit replaces some of the explicitly imported types from the
`typing` module with their inbuilt counterparts, e.g. `typing.List`
becomes `list`. This is supported since Python 3.9 [0].

Additionally, file paths are now represented as `pathlib.Path` [1],
which also checks whether the given string is actually a valid path
when constructed.

Furthermore, the `dict`s with values of mixed types are now
represented as dataclasses [2] instead, in order to make them more
type-safe (--> allow for better linting).

Because dataclasses and `pathlib.Path`s are not JSON-serializable by
default however, a helper function is added, which allows for more
fine-grained control regarding how those objects are serialized.

[0]: https://docs.python.org/3.9/whatsnew/3.9.html#type-hinting-generics-in-standard-collections
[1]: https://docs.python.org/3.11/library/pathlib.html
[2]: https://docs.python.org/3.11/library/dataclasses.html

Signed-off-by: Max Carrara <m.carrara@proxmox.com>
---
Changes v1 --> v2:
  * rebase so patch applies onto previous patch
    (the `Tuple` import removal)

 listvms.py | 99 ++++++++++++++++++++++++++++++++++++++++--------------
 1 file changed, 73 insertions(+), 26 deletions(-)

diff --git a/listvms.py b/listvms.py
index 0b64b0b..fe257a4 100755
--- a/listvms.py
+++ b/listvms.py
@@ -1,16 +1,59 @@
 #!/usr/bin/python3
 
+import dataclasses
 import json
 import ssl
 import sys
 
-from typing import List, Dict, Optional
+from dataclasses import dataclass
+from pathlib import Path
+from typing import Any
 
 from pyVim.connect import SmartConnect, Disconnect
 from pyVmomi import vim
 
 
-def get_datacenter_of_vm(vm: vim.VirtualMachine) -> Optional[vim.Datacenter]:
+@dataclass
+class VmVmxInfo:
+    datastore: str
+    path: Path
+    checksum: str
+
+
+@dataclass
+class VmDiskInfo:
+    datastore: str
+    path: Path
+    capacity: int
+
+
+@dataclass
+class VmInfo:
+    config: VmVmxInfo
+    disks: list[VmDiskInfo]
+    power: str
+
+
+def json_dump_helper(obj: Any) -> Any:
+    """Converts otherwise unserializable objects to types that can be
+    serialized as JSON.
+
+    Raises:
+        TypeError: If the conversion of the object is not supported.
+    """
+    if dataclasses.is_dataclass(obj):
+        return dataclasses.asdict(obj)
+
+    match obj:
+        case Path():
+            return str(obj)
+
+    raise TypeError(
+        f"Can't make object of type {type(obj)} JSON-serializable: {repr(obj)}"
+    )
+
+
+def get_datacenter_of_vm(vm: vim.VirtualMachine) -> vim.Datacenter | None:
     """Find the Datacenter object a VM belongs to."""
     current = vm.parent
     while current:
@@ -20,10 +63,10 @@ def get_datacenter_of_vm(vm: vim.VirtualMachine) -> Optional[vim.Datacenter]:
     return None
 
 
-def list_vms(service_instance: vim.ServiceInstance) -> List[vim.VirtualMachine]:
+def list_vms(service_instance: vim.ServiceInstance) -> list[vim.VirtualMachine]:
     """List all VMs on the ESXi/vCenter server."""
     content = service_instance.content
-    vm_view = content.viewManager.CreateContainerView(
+    vm_view: Any = content.viewManager.CreateContainerView(
         content.rootFolder,
         [vim.VirtualMachine],
         True,
@@ -32,39 +75,36 @@ def list_vms(service_instance: vim.ServiceInstance) -> List[vim.VirtualMachine]:
     vm_view.Destroy()
     return vms
 
-def parse_file_path(path):
+def parse_file_path(path) -> tuple[str, Path]:
     """Parse a path of the form '[datastore] file/path'"""
     datastore_name, relative_path = path.split('] ', 1)
     datastore_name = datastore_name.strip('[')
-    return (datastore_name, relative_path)
+    return (datastore_name, Path(relative_path))
 
-def get_vm_vmx_info(vm: vim.VirtualMachine) -> Dict[str, str]:
+def get_vm_vmx_info(vm: vim.VirtualMachine) -> VmVmxInfo:
     """Extract VMX file path and checksum from a VM object."""
     datastore_name, relative_vmx_path = parse_file_path(vm.config.files.vmPathName)
-    return {
-        'datastore': datastore_name,
-        'path': relative_vmx_path,
-        'checksum': vm.config.vmxConfigChecksum.hex() if vm.config.vmxConfigChecksum else 'N/A'
-    }
 
-def get_vm_disk_info(vm: vim.VirtualMachine) -> Dict[str, int]:
+    return VmVmxInfo(
+        datastore=datastore_name,
+        path=relative_vmx_path,
+        checksum=vm.config.vmxConfigChecksum.hex() if vm.config.vmxConfigChecksum else 'N/A'
+    )
+
+def get_vm_disk_info(vm: vim.VirtualMachine) -> list[VmDiskInfo]:
     disks = []
     for device in vm.config.hardware.device:
-        if type(device).__name__ == 'vim.vm.device.VirtualDisk':
+        if isinstance(device, vim.vm.device.VirtualDisk):
             try:
                 (datastore, path) = parse_file_path(device.backing.fileName)
                 capacity = device.capacityInBytes
-                disks.append({
-                    'datastore': datastore,
-                    'path': path,
-                    'capacity': capacity,
-                })
+                disks.append(VmDiskInfo(datastore, path, capacity))
             except Exception as err:
                 # if we can't figure out the disk stuff that's fine...
                 print("failed to get disk information for esxi vm: ", err, file=sys.stderr)
     return disks
 
-def get_all_datacenters(service_instance: vim.ServiceInstance) -> List[vim.Datacenter]:
+def get_all_datacenters(service_instance: vim.ServiceInstance) -> list[vim.Datacenter]:
     """Retrieve all datacenters from the ESXi/vCenter server."""
     content = service_instance.content
     dc_view = content.viewManager.CreateContainerView(content.rootFolder, [vim.Datacenter], True)
@@ -110,18 +150,25 @@ def main():
             name = 'vm ' + vm.name
             try:
                 dc = get_datacenter_of_vm(vm)
-                vm_info = {
-                    'config': get_vm_vmx_info(vm),
-                    'disks': get_vm_disk_info(vm),
-                    'power': vm.runtime.powerState,
-                }
+                if dc is None:
+                    print(
+                        f"Failed to get datacenter for {name}",
+                        file=sys.stderr
+                    )
+
+                vm_info = VmInfo(
+                    config=get_vm_vmx_info(vm),
+                    disks=get_vm_disk_info(vm),
+                    power=vm.runtime.powerState,
+                )
+
                 datastore_info = {ds.name: ds.url for ds in vm.config.datastoreUrl}
                 data.setdefault(dc.name, {}).setdefault('vms', {})[vm.name] = vm_info
                 data.setdefault(dc.name, {}).setdefault('datastores', {}).update(datastore_info)
             except Exception as err:
                 print("failed to get info for", name, ':', err, file=sys.stderr)
 
-        print(json.dumps(data, indent=2))
+        print(json.dumps(data, indent=2, default=json_dump_helper))
     finally:
         Disconnect(si)
 
-- 
2.39.2





^ permalink raw reply	[flat|nested] 9+ messages in thread

* [pve-devel] [PATCH v2 pve-esxi-import-tools 4/7] listvms: add arg parser, context manager for connections, fetch helper
  2024-03-22 18:06 [pve-devel] [PATCH v2 pve-esxi-import-tools 0/7] Improve listvms.py Max Carrara
                   ` (2 preceding siblings ...)
  2024-03-22 18:06 ` [pve-devel] [PATCH v2 pve-esxi-import-tools 3/7] listvms: improve typing and add dataclasses to represent dicts Max Carrara
@ 2024-03-22 18:06 ` Max Carrara
  2024-03-22 18:06 ` [pve-devel] [PATCH v2 pve-esxi-import-tools 5/7] listvms: dump json directly to stdout Max Carrara
                   ` (3 subsequent siblings)
  7 siblings, 0 replies; 9+ messages in thread
From: Max Carrara @ 2024-03-22 18:06 UTC (permalink / raw)
  To: pve-devel

In order to make the CLI interface more friendly to humans, Python's
`argparse` [0] module from the standard library is used to parse the
arguments provided to the script. Each option and positional argument
also contain a short help text that is shown when running the script
with either "-h" or "--help".

Additionally, this commit also adds a context manager [1] for
establishing connections to an ESXi host. The context manager ensures
that the connection is closed in its inner `finally` block.

The inner part of the VM-data-fetching loop in `main()` is factored
out into a separate helper function, which now raises a `RuntimeError`
if the datacenter of a VM cannot be looked up.

In general, should any exception be thrown inside the loop, its output
is subsequently logged to stderr. The loop then just continues like
before.

Any exception that is not caught inside of `main()` is now printed to
stderr, followed by exiting with `1`.

Overall, the script's behaviour and output on successful operations
remains the same, except regarding unsuccessful argument parsing and
displaying error messages. In other words, invocations prior to this
patch should result in the same JSON output (if successful).

This was tested by piping the outputs of this script before and after
this commit through `jq` and then comparing the outputs with `diff`.

[0]: https://docs.python.org/3.11/library/argparse.html
[1]: https://docs.python.org/3/library/contextlib.html#contextlib.contextmanager

Signed-off-by: Max Carrara <m.carrara@proxmox.com>
---
Changes v1 --> v2:
  * rebase onto master
  * use `Generator` as return type for the ESXi connection context
    manager
  * do not strip all whitespace from the read password file and retain
    original behaviour of only removing a trailing newline

 listvms.py | 195 ++++++++++++++++++++++++++++++++++++++---------------
 1 file changed, 142 insertions(+), 53 deletions(-)

diff --git a/listvms.py b/listvms.py
index fe257a4..354844b 100755
--- a/listvms.py
+++ b/listvms.py
@@ -1,18 +1,113 @@
 #!/usr/bin/python3
 
+import argparse
 import dataclasses
 import json
 import ssl
 import sys
+import textwrap
 
+from contextlib import contextmanager
 from dataclasses import dataclass
 from pathlib import Path
-from typing import Any
+from typing import Any, Generator
 
 from pyVim.connect import SmartConnect, Disconnect
 from pyVmomi import vim
 
 
+def parse_args() -> argparse.Namespace:
+    parser = argparse.ArgumentParser(
+        prog="listvms",
+        description="List VMs on an ESXi host.",
+    )
+
+    def _squeeze_and_wrap(text: str) -> str:
+        """Makes it easier to write help text using multiline strings."""
+        text = " ".join(text.split())
+
+        return "\n".join(textwrap.wrap(text, 60, break_on_hyphens=False))
+
+    parser.add_argument(
+        "--skip-cert-verification",
+        help=_squeeze_and_wrap(
+            """Skip the verification of TLS certs, e.g. to allow self-signed
+            certs."""
+        ),
+        action="store_true",
+    )
+
+    parser.add_argument(
+        "hostname",
+        help=_squeeze_and_wrap("""The name or address of the ESXi host."""),
+    )
+
+    parser.add_argument(
+        "username",
+        help=_squeeze_and_wrap("""The name of the user to connect with."""),
+    )
+
+    parser.add_argument(
+        "password_file",
+        help=_squeeze_and_wrap(
+            """The file which contains the password for the provided
+            username."""
+        ),
+        type=Path,
+    )
+
+    return parser.parse_args()
+
+
+@dataclass
+class EsxiConnectonArgs:
+    hostname: str
+    username: str
+    password_file: Path
+    skip_cert_verification: bool = False
+
+
+@contextmanager
+def connect_to_esxi_host(
+    args: EsxiConnectonArgs,
+) -> Generator[vim.ServiceInstance, None, None]:
+    """Opens a connection to an ESXi host with the given username and password
+    contained in the password file.
+    """
+    ssl_context = (
+        ssl._create_unverified_context()
+        if args.skip_cert_verification
+        else None
+    )
+
+    with open(args.password_file) as pw_file:
+        password = pw_file.read()
+        if password.endswith("\n"):
+            password = password[:-1]
+
+    connection = None
+
+    try:
+        connection = SmartConnect(
+            host=args.hostname,
+            user=args.username,
+            pwd=password,
+            sslContext=ssl_context,
+        )
+
+        yield connection
+
+    except ssl.SSLCertVerificationError:
+        raise ConnectionError(
+            "Failed to verify certificate - add the CA of your ESXi to the "
+            "system trust store or skip verification",
+        )
+
+    finally:
+        if connection is not None:
+            Disconnect(connection)
+
+
 @dataclass
 class VmVmxInfo:
     datastore: str
@@ -112,65 +207,59 @@ def get_all_datacenters(service_instance: vim.ServiceInstance) -> list[vim.Datac
     dc_view.Destroy()
     return datacenters
 
+
+def fetch_and_update_vm_data(vm: vim.VirtualMachine, data: dict[Any, Any]):
+    """Fetches all required VM, datastore and datacenter information, and
+    then updates the given `dict`.
+
+    Raises:
+        RuntimeError: If looking up the datacenter for the given VM fails.
+    """
+    datacenter = get_datacenter_of_vm(vm)
+    if datacenter is None:
+        raise RuntimeError(f"Failed to lookup datacenter for VM {vm.name}")
+
+    data.setdefault(datacenter.name, {})
+
+    vms = data[datacenter.name].setdefault("vms", {})
+    datastores = data[datacenter.name].setdefault("datastores", {})
+
+    vms[vm.name] = VmInfo(
+        config=get_vm_vmx_info(vm),
+        disks=get_vm_disk_info(vm),
+        power=str(vm.runtime.powerState),
+    )
+
+    datastores.update({ds.name: ds.url for ds in vm.config.datastoreUrl})
+
+
 def main():
-    if sys.argv[1] == '--skip-cert-verification':
-        del sys.argv[1]
-        ssl_context = ssl._create_unverified_context()
-    else:
-        ssl_context = None
-
-    esxi_host = sys.argv[1]
-    esxi_user = sys.argv[2]
-    esxi_password_file = sys.argv[3]
-
-    esxi_password = ''
-    with open(esxi_password_file) as f:
-        esxi_password = f.read()
-        if esxi_password.endswith('\n'):
-            esxi_password = esxi_password[:-1]
+    args = parse_args()
 
-    try:
-        si = SmartConnect(
-            host=esxi_host,
-            user=esxi_user,
-            pwd=esxi_password,
-            sslContext=ssl_context,
-        )
-    except ssl.SSLCertVerificationError as err:
-        print("failed to verify certificate - add the CA of your ESXi to the system trust store or skip verification", file=sys.stderr)
-        sys.exit(1)
-    except Exception as err:
-        print(f"failed to connect: {err}", file=sys.stderr)
-        sys.exit(1)
+    connection_args = EsxiConnectonArgs(
+        hostname=args.hostname,
+        username=args.username,
+        password_file=args.password_file,
+        skip_cert_verification=args.skip_cert_verification,
+    )
 
-    try:
-        vms = list_vms(si)
+    with connect_to_esxi_host(connection_args) as connection:
         data = {}
-        for vm in vms:
-            name = 'vm ' + vm.name
+        for vm in list_vms(connection):
             try:
-                dc = get_datacenter_of_vm(vm)
-                if dc is None:
-                    print(
-                        f"Failed to get datacenter for {name}",
-                        file=sys.stderr
-                    )
-
-                vm_info = VmInfo(
-                    config=get_vm_vmx_info(vm),
-                    disks=get_vm_disk_info(vm),
-                    power=vm.runtime.powerState,
+                fetch_and_update_vm_data(vm, data)
+            except Exception as err:
+                print(
+                    f"Failed to get info for VM {vm.name}: {err}",
+                    file=sys.stderr,
                 )
 
-                datastore_info = {ds.name: ds.url for ds in vm.config.datastoreUrl}
-                data.setdefault(dc.name, {}).setdefault('vms', {})[vm.name] = vm_info
-                data.setdefault(dc.name, {}).setdefault('datastores', {}).update(datastore_info)
-            except Exception as err:
-                print("failed to get info for", name, ':', err, file=sys.stderr)
+    print(json.dumps(data, indent=2, default=json_dump_helper))
 
-        print(json.dumps(data, indent=2, default=json_dump_helper))
-    finally:
-        Disconnect(si)
 
 if __name__ == "__main__":
-    main()
+    try:
+        main()
+    except Exception as err:
+        print(f"Encountered unexpected error: {err}", file=sys.stderr)
+        sys.exit(1)
-- 
2.39.2





^ permalink raw reply	[flat|nested] 9+ messages in thread

* [pve-devel] [PATCH v2 pve-esxi-import-tools 5/7] listvms: dump json directly to stdout
  2024-03-22 18:06 [pve-devel] [PATCH v2 pve-esxi-import-tools 0/7] Improve listvms.py Max Carrara
                   ` (3 preceding siblings ...)
  2024-03-22 18:06 ` [pve-devel] [PATCH v2 pve-esxi-import-tools 4/7] listvms: add arg parser, context manager for connections, fetch helper Max Carrara
@ 2024-03-22 18:06 ` Max Carrara
  2024-03-22 18:06 ` [pve-devel] [PATCH v2 pve-esxi-import-tools 6/7] listvms: run formatter Max Carrara
                   ` (2 subsequent siblings)
  7 siblings, 0 replies; 9+ messages in thread
From: Max Carrara @ 2024-03-22 18:06 UTC (permalink / raw)
  To: pve-devel

Signed-off-by: Max Carrara <m.carrara@proxmox.com>
---
Changes v1 --> v2:
  * new (thanks for the suggestion, Lukas!)

 listvms.py | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/listvms.py b/listvms.py
index 354844b..58b09df 100755
--- a/listvms.py
+++ b/listvms.py
@@ -254,7 +254,7 @@ def main():
                     file=sys.stderr,
                 )
 
-    print(json.dumps(data, indent=2, default=json_dump_helper))
+    json.dump(data, sys.stdout, indent=2, default=json_dump_helper)
 
 
 if __name__ == "__main__":
-- 
2.39.2





^ permalink raw reply	[flat|nested] 9+ messages in thread

* [pve-devel] [PATCH v2 pve-esxi-import-tools 6/7] listvms: run formatter
  2024-03-22 18:06 [pve-devel] [PATCH v2 pve-esxi-import-tools 0/7] Improve listvms.py Max Carrara
                   ` (4 preceding siblings ...)
  2024-03-22 18:06 ` [pve-devel] [PATCH v2 pve-esxi-import-tools 5/7] listvms: dump json directly to stdout Max Carrara
@ 2024-03-22 18:06 ` Max Carrara
  2024-03-22 18:06 ` [pve-devel] [PATCH v2 pve-esxi-import-tools 7/7] use mypy for automatic type checks in Python Max Carrara
  2024-03-27 10:50 ` [pve-devel] applied-series: [PATCH v2 pve-esxi-import-tools 0/7] Improve listvms.py Wolfgang Bumiller
  7 siblings, 0 replies; 9+ messages in thread
From: Max Carrara @ 2024-03-22 18:06 UTC (permalink / raw)
  To: pve-devel

This commit formats the script using `black -l 80` [0], even though we
don't have an official style guide for Python.

[0]: https://github.com/psf/black

Signed-off-by: Max Carrara <m.carrara@proxmox.com>
---
Changes v1 --> v2:
  * none

 listvms.py | 30 +++++++++++++++++++++++-------
 1 file changed, 23 insertions(+), 7 deletions(-)

diff --git a/listvms.py b/listvms.py
index 58b09df..90b27bf 100755
--- a/listvms.py
+++ b/listvms.py
@@ -170,22 +170,29 @@ def list_vms(service_instance: vim.ServiceInstance) -> list[vim.VirtualMachine]:
     vm_view.Destroy()
     return vms
 
+
 def parse_file_path(path) -> tuple[str, Path]:
     """Parse a path of the form '[datastore] file/path'"""
-    datastore_name, relative_path = path.split('] ', 1)
-    datastore_name = datastore_name.strip('[')
+    datastore_name, relative_path = path.split("] ", 1)
+    datastore_name = datastore_name.strip("[")
     return (datastore_name, Path(relative_path))
 
+
 def get_vm_vmx_info(vm: vim.VirtualMachine) -> VmVmxInfo:
     """Extract VMX file path and checksum from a VM object."""
-    datastore_name, relative_vmx_path = parse_file_path(vm.config.files.vmPathName)
+    datastore_name, relative_vmx_path = parse_file_path(
+        vm.config.files.vmPathName
+    )
 
     return VmVmxInfo(
         datastore=datastore_name,
         path=relative_vmx_path,
-        checksum=vm.config.vmxConfigChecksum.hex() if vm.config.vmxConfigChecksum else 'N/A'
+        checksum=vm.config.vmxConfigChecksum.hex()
+        if vm.config.vmxConfigChecksum
+        else "N/A",
     )
 
+
 def get_vm_disk_info(vm: vim.VirtualMachine) -> list[VmDiskInfo]:
     disks = []
     for device in vm.config.hardware.device:
@@ -196,13 +203,22 @@ def get_vm_disk_info(vm: vim.VirtualMachine) -> list[VmDiskInfo]:
                 disks.append(VmDiskInfo(datastore, path, capacity))
             except Exception as err:
                 # if we can't figure out the disk stuff that's fine...
-                print("failed to get disk information for esxi vm: ", err, file=sys.stderr)
+                print(
+                    "failed to get disk information for esxi vm: ",
+                    err,
+                    file=sys.stderr,
+                )
     return disks
 
-def get_all_datacenters(service_instance: vim.ServiceInstance) -> list[vim.Datacenter]:
+
+def get_all_datacenters(
+    service_instance: vim.ServiceInstance,
+) -> list[vim.Datacenter]:
     """Retrieve all datacenters from the ESXi/vCenter server."""
     content = service_instance.content
-    dc_view = content.viewManager.CreateContainerView(content.rootFolder, [vim.Datacenter], True)
+    dc_view: Any = content.viewManager.CreateContainerView(
+        content.rootFolder, [vim.Datacenter], True
+    )
     datacenters = dc_view.view
     dc_view.Destroy()
     return datacenters
-- 
2.39.2





^ permalink raw reply	[flat|nested] 9+ messages in thread

* [pve-devel] [PATCH v2 pve-esxi-import-tools 7/7] use mypy for automatic type checks in Python
  2024-03-22 18:06 [pve-devel] [PATCH v2 pve-esxi-import-tools 0/7] Improve listvms.py Max Carrara
                   ` (5 preceding siblings ...)
  2024-03-22 18:06 ` [pve-devel] [PATCH v2 pve-esxi-import-tools 6/7] listvms: run formatter Max Carrara
@ 2024-03-22 18:06 ` Max Carrara
  2024-03-27 10:50 ` [pve-devel] applied-series: [PATCH v2 pve-esxi-import-tools 0/7] Improve listvms.py Wolfgang Bumiller
  7 siblings, 0 replies; 9+ messages in thread
From: Max Carrara @ 2024-03-22 18:06 UTC (permalink / raw)
  To: pve-devel

This commit adds mypy [0] as build dependency and ensures it is
invoked during the package build process.

mypy can also be manually invoked via `make lint`.

A mypy.ini file [1] is also added to disable errors regarding missing
type stubs for pyVmomi and pyVim.

[0]: https://www.mypy-lang.org/
[1]: https://mypy.readthedocs.io/en/stable/config_file.html

Signed-off-by: Max Carrara <m.carrara@proxmox.com>
---
Changes v1 --> v2:
  * new (thanks for suggesting to add mypy.ini, Lukas!)

 Makefile       | 13 ++++++++++++-
 debian/control |  1 +
 mypy.ini       |  8 ++++++++
 3 files changed, 21 insertions(+), 1 deletion(-)
 create mode 100644 mypy.ini

diff --git a/Makefile b/Makefile
index 73e7c5e..cc70305 100644
--- a/Makefile
+++ b/Makefile
@@ -28,6 +28,7 @@ BINARY = $(COMPILEDIR)/esxi-folder-fuse
 SCRIPT = listvms.py
 
 CARGO ?= cargo
+MYPY ?= mypy
 
 .PHONY: all
 all: $(BINARY)
@@ -40,8 +41,17 @@ check: test
 test:
 	$(CARGO) test $(CARGO_BUILD_ARGS)
 
+.lint-incremental: $(SCRIPT)
+	$(MYPY) $?
+	touch "$@"
+
+.PHONY: lint
+lint: $(SCRIPT)
+	$(MYPY) $(SCRIPT)
+	touch ".lint-incremental"
+
 .PHONY: install
-install: $(BINARY) $(SCRIPT)
+install: $(BINARY) $(SCRIPT) .lint-incremental
 	install -m755 -d $(DESTDIR)$(LIBEXECDIR)/pve-esxi-import-tools
 	install -m755 -t $(DESTDIR)$(LIBEXECDIR)/pve-esxi-import-tools $(BINARY)
 	install -m755 -t $(DESTDIR)$(LIBEXECDIR)/pve-esxi-import-tools $(SCRIPT)
@@ -55,6 +65,7 @@ $(BUILD_DIR):
 	cp -t $@.tmp -a \
 	  debian \
 	  Makefile \
+	  mypy.ini \
 	  listvms.py \
 	  Cargo.toml \
 	  src
diff --git a/debian/control b/debian/control
index 3c12f29..8687d6d 100644
--- a/debian/control
+++ b/debian/control
@@ -30,6 +30,7 @@ Build-Depends: cargo:native (>= 0.65.0~),
                librust-tokio-1+rt-multi-thread-dev,
                librust-tokio-1+time-dev,
                libstd-rust-dev,
+               mypy,
                rustc:native,
 Maintainer: Proxmox Support Team <support@proxmox.com>
 Standards-Version: 4.6.2
diff --git a/mypy.ini b/mypy.ini
new file mode 100644
index 0000000..e6724c8
--- /dev/null
+++ b/mypy.ini
@@ -0,0 +1,8 @@
+[mypy]
+
+[mypy-pyVmomi]
+ignore_missing_imports = True
+
+[mypy-pyVim.*]
+ignore_missing_imports = True
+
-- 
2.39.2





^ permalink raw reply	[flat|nested] 9+ messages in thread

* [pve-devel] applied-series: [PATCH v2 pve-esxi-import-tools 0/7] Improve listvms.py
  2024-03-22 18:06 [pve-devel] [PATCH v2 pve-esxi-import-tools 0/7] Improve listvms.py Max Carrara
                   ` (6 preceding siblings ...)
  2024-03-22 18:06 ` [pve-devel] [PATCH v2 pve-esxi-import-tools 7/7] use mypy for automatic type checks in Python Max Carrara
@ 2024-03-27 10:50 ` Wolfgang Bumiller
  7 siblings, 0 replies; 9+ messages in thread
From: Wolfgang Bumiller @ 2024-03-27 10:50 UTC (permalink / raw)
  To: Max Carrara; +Cc: pve-devel

applied series, thanks




^ permalink raw reply	[flat|nested] 9+ messages in thread

end of thread, other threads:[~2024-03-27 10:50 UTC | newest]

Thread overview: 9+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2024-03-22 18:06 [pve-devel] [PATCH v2 pve-esxi-import-tools 0/7] Improve listvms.py Max Carrara
2024-03-22 18:06 ` [pve-devel] [PATCH v2 pve-esxi-import-tools 1/7] listvms: remove unused import and variable Max Carrara
2024-03-22 18:06 ` [pve-devel] [PATCH v2 pve-esxi-import-tools 2/7] listvms: reorder imports Max Carrara
2024-03-22 18:06 ` [pve-devel] [PATCH v2 pve-esxi-import-tools 3/7] listvms: improve typing and add dataclasses to represent dicts Max Carrara
2024-03-22 18:06 ` [pve-devel] [PATCH v2 pve-esxi-import-tools 4/7] listvms: add arg parser, context manager for connections, fetch helper Max Carrara
2024-03-22 18:06 ` [pve-devel] [PATCH v2 pve-esxi-import-tools 5/7] listvms: dump json directly to stdout Max Carrara
2024-03-22 18:06 ` [pve-devel] [PATCH v2 pve-esxi-import-tools 6/7] listvms: run formatter Max Carrara
2024-03-22 18:06 ` [pve-devel] [PATCH v2 pve-esxi-import-tools 7/7] use mypy for automatic type checks in Python Max Carrara
2024-03-27 10:50 ` [pve-devel] applied-series: [PATCH v2 pve-esxi-import-tools 0/7] Improve listvms.py Wolfgang Bumiller

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox
Service provided by Proxmox Server Solutions GmbH | Privacy | Legal