Copilot commented on code in PR #13832: URL: https://github.com/apache/cloudstack/pull/13832#discussion_r3743258311
########## extensions/Proxmox/proxmox.py: ########## @@ -0,0 +1,697 @@ +#!/usr/bin/env python3 +# Licensed to the Apache Software Foundation (ASF) under one +# or more contributor license agreements. See the NOTICE file Review Comment: This PR removes `extensions/Proxmox/proxmox.sh` but other parts of the repo still reference/install/register that path (e.g. `engine/schema/.../schema-42040to42100.sql:480` registers `Proxmox/proxmox.sh`, and `debian/cloudstack-management.install:25` installs `/etc/cloudstack/extensions/Proxmox/proxmox.sh`). Without updating those references to `Proxmox/proxmox.py`, the Proxmox extension will not be discoverable/packaged correctly. ########## extensions/Proxmox/proxmox.py: ########## @@ -0,0 +1,697 @@ +#!/usr/bin/env python3 +# Licensed to the Apache Software Foundation (ASF) under one +# or more contributor license agreements. See the NOTICE file +# distributed with this work for additional information +# regarding copyright ownership. The ASF licenses this file +# to you under the Apache License, Version 2.0 (the +# "License"); you may not use this file except in compliance +# with the License. You may obtain a copy of the License at +# +# http://www.apache.org/licenses/LICENSE-2.0 +# +# Unless required by applicable law or agreed to in writing, +# software distributed under the License is distributed on an +# "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY +# KIND, either express or implied. See the License for the +# specific language governing permissions and limitations +# under the License. +from __future__ import annotations +import datetime as _dt +import json +import re +import ssl +import sys +import time +from dataclasses import dataclass +from pathlib import Path +from typing import Any +from urllib import error, parse, request + +DEFAULT_WAIT_SECONDS = 600 +PROXMOX_API_PORT = 8006 +PROXMOX_API_PREFIX = "/api2/json" +VM_NAME_PATTERN = re.compile(r"^[a-zA-Z0-9-]+$") + + +class ProxmoxError(RuntimeError): + """Raised when the Proxmox API or payload validation fails.""" + + +def fail(message: str) -> None: + print(json.dumps({"status": "error", "error": message})) + raise SystemExit(1) + + +def succeed(data: dict[str, Any]) -> None: + print(json.dumps(data)) + raise SystemExit(0) + + +def _is_mapping(value: Any) -> bool: + return isinstance(value, dict) + + +def _mapping(value: Any) -> dict[str, Any]: + return value if isinstance(value, dict) else {} + + +def _string(value: Any, default: str = "") -> str: + if value is None: + return default + if isinstance(value, str): + return value + return str(value) + + +def _bool_text(value: Any, default: bool = False) -> bool: + if value is None: + return default + if isinstance(value, bool): + return value + return _string(value).strip().lower() in {"1", "true", "yes", "on"} + + +def _int_text(value: Any, default: int = 0) -> int: + if value is None or value == "": + return default + try: + return int(value) + except (TypeError, ValueError): + return default + + +def _normalize_url(url: str) -> str: + url = url.strip() + if not url.startswith(("http://";, "https://";)): + url = "https://"; + url + return url.rstrip("/") + + +def _format_snapshot_time(value: Any) -> str: + if value in (None, "", "-"): + return "-" + try: + return _dt.datetime.fromtimestamp(int(float(value))).strftime( + "%Y-%m-%d %H:%M:%S" + ) + except (TypeError, ValueError, OSError, OverflowError): + return _string(value, "-") + + +@dataclass(slots=True) +class ProxmoxSettings: + url: str + user: str + token: str + secret: str + node: str + network_bridge: str + verify_tls_certificate: bool + vm_name: str + vm_internal_name: str + vmid: str + vmcpus: int + vmmemory: int + template_type: str + template_id: str + iso_path: str + iso_os_type: str + disk_size_gb: str + storage: str + is_full_clone: bool + snap_name: str + snap_description: str + snap_save_memory: bool + mac_addresses: list[str] + vlans: list[str] + + +class ProxmoxManager: + def __init__(self, config_path: str, wait_time: int | None = None): + self.config_path = config_path + self.wait_time = ( + wait_time if wait_time and wait_time > 0 else DEFAULT_WAIT_SECONDS Review Comment: `proxmox.py` uses Python 3.10+ syntax/features (e.g. `wait_time: int | None` and other PEP 604 unions; `@dataclass(slots=True)`; `zip(..., strict=False)`). CloudStack packaging/tests reference Python 3.6 compatibility in other components (e.g. `packaging/el8/cloud.spec:342` and `scripts/vm/.../test_base.py:63`), so this script may fail to even start on supported distros unless the minimum Python is raised consistently. ########## extensions/Proxmox/proxmox.py: ########## @@ -0,0 +1,697 @@ +#!/usr/bin/env python3 +# Licensed to the Apache Software Foundation (ASF) under one +# or more contributor license agreements. See the NOTICE file +# distributed with this work for additional information +# regarding copyright ownership. The ASF licenses this file +# to you under the Apache License, Version 2.0 (the +# "License"); you may not use this file except in compliance +# with the License. You may obtain a copy of the License at +# +# http://www.apache.org/licenses/LICENSE-2.0 +# +# Unless required by applicable law or agreed to in writing, +# software distributed under the License is distributed on an +# "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY +# KIND, either express or implied. See the License for the +# specific language governing permissions and limitations +# under the License. +from __future__ import annotations +import datetime as _dt +import json +import re +import ssl +import sys +import time +from dataclasses import dataclass +from pathlib import Path +from typing import Any +from urllib import error, parse, request + +DEFAULT_WAIT_SECONDS = 600 +PROXMOX_API_PORT = 8006 +PROXMOX_API_PREFIX = "/api2/json" +VM_NAME_PATTERN = re.compile(r"^[a-zA-Z0-9-]+$") + + +class ProxmoxError(RuntimeError): + """Raised when the Proxmox API or payload validation fails.""" + + +def fail(message: str) -> None: + print(json.dumps({"status": "error", "error": message})) + raise SystemExit(1) + + +def succeed(data: dict[str, Any]) -> None: + print(json.dumps(data)) + raise SystemExit(0) + + +def _is_mapping(value: Any) -> bool: + return isinstance(value, dict) + Review Comment: `_is_mapping()` is defined but never used, which adds dead code and can confuse future maintenance. -- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. To unsubscribe, e-mail: [email protected] For queries about this service, please contact Infrastructure at: [email protected]
