Skip to content

Commit

Permalink
fix: multiple fixes and improvements
Browse files Browse the repository at this point in the history
Fixes several issues:

- installation is now performed in a background thread to let tkinter thread update the view
- on windows, COM needs to be initialized in the working thread in order to
  create the shortcut
- remove cancel button from final frame
- improve some logs
- wait 60s for server startup on windows
- adds a popup with stacktrace when an uncaught exception occurs
- fix healthcheck URL
- fix deadlock when waiting for server start fails 

Also adds configuration migration for v2.18:
a new api_prefix field is required
  • Loading branch information
sylvlecl authored Oct 11, 2024
2 parents 2b18254 + d84a6ea commit eda9075
Show file tree
Hide file tree
Showing 17 changed files with 225 additions and 115 deletions.
5 changes: 2 additions & 3 deletions pyproject.toml
Original file line number Diff line number Diff line change
Expand Up @@ -31,7 +31,7 @@ dependencies = [
'psutil<5.10',
'pyyaml<6.1',
'pywin32<=306; sys_platform == "win32"',
'httpx<0.28',
'requests',
'platformdirs<=4.2.2',
'uvicorn',
]
Expand All @@ -52,7 +52,6 @@ dependencies = [
"pytest-datadir",
"fastapi",
"pyinstaller",
"httpx==0.27.0",
"requests",
"platformdirs==4.2.2",
"uvicorn",
Expand All @@ -79,7 +78,7 @@ dependencies = [
"mypy>=1.0.0",
"fastapi",
"pyinstaller",
"httpx==0.27.0"
"requests"
]
[tool.hatch.envs.types.scripts]
check = "mypy --install-types --non-interactive {args:src/antares_web_installer tests}"
Expand Down
Empty file added pytest.ini
Empty file.
104 changes: 53 additions & 51 deletions src/antares_web_installer/app.py
Original file line number Diff line number Diff line change
Expand Up @@ -5,12 +5,12 @@
import textwrap
import time
import webbrowser

from difflib import SequenceMatcher
from pathlib import Path
from shutil import copy2, copytree
from typing import List

import httpx
import requests

if os.name == "nt":
from pythoncom import com_error
Expand All @@ -31,6 +31,9 @@
SHORTCUT_NAMES = {"posix": "AntaresWebServer.desktop", "nt": "AntaresWebServer.lnk"}

SERVER_ADDRESS = "http://127.0.0.1:8080"
HEALTHCHECK_ADDRESS = f"{SERVER_ADDRESS}/api/health"

MAX_SERVER_START_TIME = 60


class InstallError(Exception):
Expand All @@ -54,7 +57,7 @@ class App:
def __post_init__(self):
# Prepare the path to the executable which is located in the target directory
server_name = SERVER_NAMES[os.name]
self.server_path = self.target_dir.joinpath("AntaresWeb", server_name)
self.server_path = self.target_dir / "AntaresWeb" / server_name

# Set all progress variables needed to compute current progress of the installation
self.nb_steps = 2 # kill, install steps
Expand Down Expand Up @@ -95,35 +98,43 @@ def kill_running_server(self) -> None:
Check whether Antares service is up.
Kill the process if so.
"""
processes_list = list(psutil.process_iter(["pid", "name"]))
processes_list_length = len(processes_list)
server_processes = self._get_server_processes()
if len(server_processes) > 0:
logger.info("Attempt to stop running Antares server ...")
for p in server_processes:
try:
p.kill()
except psutil.NoSuchProcess:
logger.debug(f"The process '{p.pid}' was stopped before being killed.")
continue
gone, alive = psutil.wait_procs(server_processes, timeout=5)
alive_count = len(alive)
if alive_count > 0:
raise InstallError(
"Could not to stop Antares server. Please stop it before launching again the installation."
)
else:
logger.info("Antares server successfully stopped...")
else:
logger.info("No running server found, resuming installation.")
self.update_progress(100)

for index, proc in enumerate(processes_list):
# evaluate matching between query process name and existing process name
def _get_server_processes(self) -> List[psutil.Process]:
res = []
for process in psutil.process_iter(["pid", "name"]):
try:
matching_ratio = SequenceMatcher(None, "antareswebserver", proc.name().lower()).ratio()
# evaluate matching between query process name and existing process name
matching_ratio = SequenceMatcher(None, "antareswebserver", process.name().lower()).ratio()
except FileNotFoundError:
logger.warning("The process '{}' does not exist anymore. Skipping its analysis".format(proc.name()))
logger.warning("The process '{}' does not exist anymore. Skipping its analysis".format(process.name()))
continue
except psutil.NoSuchProcess:
logger.warning("The process '{}' was stopped before being analyzed. Skipping.".format(proc.name()))
logger.warning("The process '{}' was stopped before being analyzed. Skipping.".format(process.name()))
continue
if matching_ratio > 0.8:
logger.info("Running server found. Attempt to stop it ...")
logger.debug(f"Server process:{proc.name()} - process id: {proc.pid}")
running_app = psutil.Process(pid=proc.pid)
running_app.kill()

try:
running_app.wait(5)
except psutil.TimeoutExpired as e:
raise InstallError(
"Impossible to kill the server. Please kill it manually before relaunching the installer."
) from e
else:
logger.info("The application was successfully stopped.")
self.update_progress((index + 1) * 100 / processes_list_length)
logger.info("No other processes found.")
res.append(process)
logger.debug(f"Running server found: {process.name()} - process id: {process.pid}")
return res

def install_files(self):
""" """
Expand Down Expand Up @@ -254,7 +265,7 @@ def create_shortcuts(self):
description="Launch Antares Web Server in background",
)
except com_error as e:
raise InstallError("Impossible to create a new shortcut: {}\nSkip shortcut creation".format(e))
raise InstallError(f"Impossible to create a new shortcut: {e}\nSkipping shortcut creation") from e
else:
assert shortcut_path in list(desktop_path.iterdir())
logger.info(f"Server shortcut {shortcut_path} was successfully created.")
Expand All @@ -270,42 +281,33 @@ def start_server(self):
args = [self.server_path]
server_process = subprocess.Popen(
args=args,
stdin=subprocess.PIPE,
stdout=subprocess.PIPE,
stderr=subprocess.STDOUT,
cwd=self.target_dir,
stdout=subprocess.DEVNULL,
stderr=subprocess.DEVNULL,
shell=True,
)
self.update_progress(50)

if not server_process.poll():
logger.info("Server is starting up ...")
else:
stdout, stderr = server_process.communicate()
msg = f"The server unexpectedly stopped running. (code {server_process.returncode})"
logger.info(msg)
logger.info(f"Server unexpectedly stopped.\nstdout: {stdout}\nstderr: {stderr}")
raise InstallError(msg)

nb_attempts = 0
max_attempts = 10

while nb_attempts < max_attempts:
logger.info(f"Attempt #{nb_attempts}...")
start_time = time.time()
nb_attempts = 1
while time.time() - start_time < MAX_SERVER_START_TIME:
logger.info(f"Waiting for server start (attempt #{nb_attempts})...")
if server_process.poll() is not None:
raise InstallError("Server failed to start, please check server logs.")
try:
res = httpx.get(SERVER_ADDRESS + "/health", timeout=1)
res = requests.get(HEALTHCHECK_ADDRESS)
if res.status_code == 200:
logger.info("The server is now running.")
break
except httpx.RequestError:
time.sleep(1)
else:
logger.debug(f"Got HTTP status code {res.status_code} while requesting {HEALTHCHECK_ADDRESS}")
logger.debug(f"Content: {res.text}")
except requests.RequestException as req_err:
logger.debug(f"Error while requesting {HEALTHCHECK_ADDRESS}: {req_err}", exc_info=req_err)
time.sleep(1)
nb_attempts += 1
else:
stdout, stderr = server_process.communicate()
msg = "The server didn't start in time"
logger.error(msg)
logger.error(f"stdout: {stdout}\nstderr: {stderr}")
raise InstallError(msg)
raise InstallError("Server didn't start in time, please check server logs.")

def open_browser(self):
"""
Expand Down
3 changes: 3 additions & 0 deletions src/antares_web_installer/config/__init__.py
Original file line number Diff line number Diff line change
Expand Up @@ -29,5 +29,8 @@ def update_config(source_path: Path, target_path: Path, version: str) -> None:
with target_path.open("r") as f:
config = yaml.safe_load(f)

if version_info < (2, 18):
update_to_2_15(config)

with source_path.open(mode="w") as f:
yaml.dump(config, f)
10 changes: 10 additions & 0 deletions src/antares_web_installer/config/config_2_18.py
Original file line number Diff line number Diff line change
@@ -0,0 +1,10 @@
import typing as t


def update_to_2_18(config: t.MutableMapping[str, t.Any]) -> None:
"""
Update the configuration file to version 2.18 in-place:
we need to ensure root_path is / and api_prefix is /api
"""
del config["root_path"]
config["api_prefix"] = "/api"
Empty file modified src/antares_web_installer/gui/__main__.py
100644 → 100755
Empty file.
54 changes: 19 additions & 35 deletions src/antares_web_installer/gui/controller.py
Original file line number Diff line number Diff line change
Expand Up @@ -6,27 +6,21 @@
import typing
from pathlib import Path
from threading import Thread

from antares_web_installer.gui.mvc import Controller, ControllerError
from antares_web_installer.gui.model import WizardModel
from antares_web_installer.gui.view import WizardView
from typing import Optional

from antares_web_installer import logger
from antares_web_installer.app import App, InstallError
from antares_web_installer.gui.logger import ConsoleHandler, ProgressHandler, LogFileHandler
from antares_web_installer.gui.model import WizardModel
from antares_web_installer.gui.mvc import Controller
from antares_web_installer.gui.view import WizardView


class InstallationThread(Thread):
def __init__(self, group=None, target=None, name=None, args=(), kwargs=None, *, daemon=None):
super().__init__(group, target, name, args, kwargs, daemon=daemon)

def run(self):
try:
super().run()
except OSError as e:
raise e
except InstallError as e:
raise e
def run_installation(app: App) -> None:
try:
app.run()
except Exception as e:
logger.exception(f"An error occurred during installation: {e}")


class WizardController(Controller):
Expand All @@ -39,8 +33,8 @@ class WizardController(Controller):
def __init__(self):
super().__init__()
self.app = None
self.log_dir = None
self.log_file = None
self.log_dir: Optional[Path] = None
self.log_file: Optional[Path] = None

# init loggers
self.logger = logger
Expand All @@ -64,17 +58,18 @@ def init_view(self) -> "WizardView":
return WizardView(self)

def init_file_handler(self):
self.log_dir = self.model.target_dir.joinpath("logs/")
self.log_dir: Path = self.model.target_dir / "logs"
tmp_file_name = "wizard.log"

if not self.log_dir.exists():
self.log_dir = self.model.source_dir.joinpath("logs/") # use the source directory as tmp dir for logs
self.log_dir = self.model.source_dir / "logs" # use the source directory as tmp dir for logs
self.logger.debug(
"No log directory found with path '{}'. Attempt to generate the path.".format(self.log_dir)
)
self.log_dir.mkdir(parents=True, exist_ok=True)
self.logger.info("Path '{}' was successfully created.".format(self.log_dir))

self.log_file = self.log_dir.joinpath(tmp_file_name)
self.log_file = self.log_dir / tmp_file_name

# check if file exists
if self.log_file not in list(self.log_dir.iterdir()):
Expand Down Expand Up @@ -140,29 +135,18 @@ def install(self, callback: typing.Callable):
logger.warning("Impossible to create a new shortcut. Skip this step.")
logger.debug(e)

thread = InstallationThread(target=self.app.run, args=())
self.thread = Thread(target=lambda: run_installation(self.app), args=())

try:
thread.run()
self.thread.start()
except InstallError as e:
self.view.raise_error(e)

def installation_over(self) -> None:
"""
This method makes sure the thread terminated. If not, it waits for it to terminate.
"""
if self.thread:
while self.thread.join():
if not self.thread.is_alive():
break

def get_target_dir(self) -> Path:
return self.model.target_dir

def set_target_dir(self, path: Path):
result = self.model.set_target_dir(path)
if not result:
raise ControllerError("Path '{}' is not a directory.".format(path))
def set_target_dir(self, path: Path) -> None:
self.model.set_target_dir(path)

def get_shortcut(self) -> bool:
return self.model.shortcut
Expand Down
7 changes: 5 additions & 2 deletions src/antares_web_installer/gui/logger.py
Original file line number Diff line number Diff line change
Expand Up @@ -2,8 +2,11 @@
import typing


MessageConsumer = typing.Callable[[str], None]


class ConsoleHandler(logging.Handler):
def __init__(self, callback: typing.Callable):
def __init__(self, callback: MessageConsumer):
logging.Handler.__init__(self)
self.setLevel(logging.INFO)
formatter = logging.Formatter("[%(asctime)-15s] %(message)s")
Expand All @@ -15,7 +18,7 @@ def emit(self, logs: logging.LogRecord):


class ProgressHandler(logging.Handler):
def __init__(self, callback: typing.Callable):
def __init__(self, callback: MessageConsumer):
"""
This logging handler intercept all logs that are progression values
@param progress_var: tkinter.StringVar
Expand Down
7 changes: 7 additions & 0 deletions src/antares_web_installer/gui/mvc.py
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,8 @@
from __future__ import annotations

import tkinter as tk
import traceback
from tkinter import messagebox


class Model:
Expand Down Expand Up @@ -35,6 +37,11 @@ class View(tk.Tk):
def __init__(self, controller: Controller):
super().__init__()
self.controller = controller
self.report_callback_exception = self.show_error

def show_error(self, *args):
err = traceback.format_exception(*args)
messagebox.showerror("Exception", "".join(err))


class ControllerError(Exception):
Expand Down
7 changes: 1 addition & 6 deletions src/antares_web_installer/gui/view.py
Original file line number Diff line number Diff line change
Expand Up @@ -154,11 +154,7 @@ def get_target_dir(self) -> Path:
return self.controller.get_target_dir()

def set_target_dir(self, new_target_dir: str):
try:
self.controller.set_target_dir(Path(new_target_dir))
except ControllerError as e:
logger.warning("Path is not valid: {}".format(e))
self.raise_warning("Path selected is not valid")
self.controller.set_target_dir(Path(new_target_dir))

def get_launch(self) -> bool:
return self.controller.get_launch()
Expand Down Expand Up @@ -188,5 +184,4 @@ def run_installation(self, callback):
self.controller.install(callback)

def installation_over(self):
self.controller.installation_over()
self.frames["progress_frame"].installation_over()
2 changes: 1 addition & 1 deletion src/antares_web_installer/gui/widgets/frame.py
Original file line number Diff line number Diff line change
Expand Up @@ -280,5 +280,5 @@ def __init__(self, master: tk.Misc, window: "WizardView", *args, **kwargs):
style="Description.TLabel",
).pack(side="top", fill="x")

self.control_btn = ControlFrame(parent=self, window=window, finish_btn=True)
self.control_btn = ControlFrame(parent=self, window=window, cancel_btn=False, finish_btn=True)
self.control_btn.pack(side="bottom", fill="x")
Loading

0 comments on commit eda9075

Please sign in to comment.