Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix frontend build nondeterminism #201

Merged
merged 3 commits into from
Jan 25, 2025
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
14 changes: 14 additions & 0 deletions controller/thymis_controller/crud/task.py
Original file line number Diff line number Diff line change
Expand Up @@ -92,3 +92,17 @@ def delete_all_tasks(db_session: Session):
if "RUNNING_IN_PLAYWRIGHT" in os.environ:
db_session.query(db_models.Task).delete()
db_session.commit()


def get_all_tasks(db_session: Session):
return db_session.query(db_models.Task).all()


def get_all_alive_tasks(db_session: Session):
# means that the task is not in any of the final states
# final states are: failed, completed
return (
db_session.query(db_models.Task)
.filter(db_models.Task.state.not_in(["failed", "completed"]))
.all()
)
15 changes: 10 additions & 5 deletions controller/thymis_controller/task/controller.py
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
import contextlib
import logging
import os
import time
from datetime import datetime

import sqlalchemy
Expand Down Expand Up @@ -107,9 +108,13 @@ def retry_task(self, task_id: str, db_session: Session):

def delete_all_tasks(self, db_session: Session):
if "RUNNING_IN_PLAYWRIGHT" in os.environ:
for task in crud.task.get_pending_tasks(db_session):
task.state = "failed"
task.add_exception("Task was running when controller was shut down")
self.executor.cancel_all_tasks()
db_session.commit()
task_ids = []
for task in crud.task.get_all_tasks(db_session):
# save their ids
task_ids.append(task.id)
# while there are still alive tasks, spam cancel them
while crud.task.get_all_alive_tasks(db_session):
for task_id in task_ids:
self.executor.cancel_task(task_id)
time.sleep(0.1)
crud.task.delete_all_tasks(db_session)
19 changes: 0 additions & 19 deletions controller/thymis_controller/task/executor.py
Original file line number Diff line number Diff line change
Expand Up @@ -96,25 +96,6 @@ def cancel_task(self, task_id: uuid.UUID):
if task_id in self.futures:
self.futures[task_id][1].send(models_task.CancelTask(id=task_id))

def cancel_all_tasks(self):
if "RUNNING_IN_PLAYWRIGHT" in os.environ:
future_tasks = list(self.futures.keys())
for task_id in future_tasks:
if (
task_id in self.futures
and self.futures[task_id][0].running()
and not self.futures[task_id][1].closed
):
try:
self.cancel_task(task_id)
except Exception as e:
logger.error("Error cancelling task %s: %s", task_id, e)
self.futures[task_id][1].send(
models_task.CancelTask(id=task_id)
)
else:
raise ValueError("cancel_all_tasks can only be called from playwright")

def listen_child_messages(self, conn: Connection, task_id: uuid.UUID):
message = None
try:
Expand Down
12 changes: 9 additions & 3 deletions controller/thymis_controller/task/subscribe_ui.py
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,7 @@
import logging
import threading

from fastapi import WebSocket
from fastapi import WebSocket, WebSocketDisconnect
from fastapi.websockets import WebSocketState
from thymis_controller import crud, db_models

Expand Down Expand Up @@ -48,9 +48,15 @@ async def send_loop(self):
task = await self.task_queue.get()

with self.subscribers_lock:
for subscriber in self.subscribers:
if subscriber.application_state == WebSocketState.CONNECTED:
subscribers = self.subscribers.copy()

for subscriber in subscribers:
if subscriber.application_state == WebSocketState.CONNECTED:
try:
await subscriber.send_json(task)
except WebSocketDisconnect:
with self.subscribers_lock:
self.subscribers.remove(subscriber)
except asyncio.QueueShutDown:
break

Expand Down
5 changes: 4 additions & 1 deletion flake.nix
Original file line number Diff line number Diff line change
Expand Up @@ -154,7 +154,9 @@
packages = eachSystem (system:
let
pkgs = nixpkgs.legacyPackages.${system};
thymis-frontend = pkgs.callPackage ./frontend { };
thymis-frontend = pkgs.callPackage ./frontend {
git-rev = inputs.self.rev or inputs.self.dirtyRev or null;
};
thymis-controller = pkgs.callPackage ./controller {
poetry2nix = (
(poetry2nix.lib.mkPoetry2Nix { inherit pkgs; })
Expand All @@ -168,6 +170,7 @@
};
in
{
thymis-frontend = thymis-frontend;
thymis-controller = thymis-controller;
thymis-controller-container = import ./docker.nix { inherit pkgs thymis-controller; };
thymis-agent = thymis-agent;
Expand Down
2 changes: 2 additions & 0 deletions frontend/default.nix
Original file line number Diff line number Diff line change
@@ -1,9 +1,11 @@
{ buildNpmPackage
, runtimeShell
, nodejs_22
, git-rev
, lib
}:
buildNpmPackage {
GIT_REV = git-rev;
pname = "thymis-frontend";
version = "0.0.1";
src = ./.;
Expand Down
4 changes: 2 additions & 2 deletions frontend/playwright.config.ts
Original file line number Diff line number Diff line change
Expand Up @@ -41,7 +41,7 @@ const commandFrame = (cmd) =>

const config: PlaywrightTestConfig = {
workers: 1, // serial execution, but not with interdependent tests, but only one application instance
retries: 5,
retries: 2,
webServer: {
command: commandFrame(command),
port: 8000,
Expand All @@ -66,7 +66,7 @@ const config: PlaywrightTestConfig = {
expect: {
toHaveScreenshot: {
maxDiffPixels: 1,
threshold: 0.01
threshold: 0.02
}
}
};
Expand Down
21 changes: 20 additions & 1 deletion frontend/svelte.config.js
Original file line number Diff line number Diff line change
@@ -1,5 +1,21 @@
import adapter from '@sveltejs/adapter-node';
import { vitePreprocess } from '@sveltejs/vite-plugin-svelte';
import { execSync } from 'child_process';

const headRev = process.env.GIT_REV || execSync('git rev-parse HEAD').toString().trim();
let dirty = false;
try {
dirty =
(process.env.GIT_REV || '').search('dirty') != -1 ||
(!((process.env.GIT_REV || '').search('dirty') != -1) &&
execSync('git status --porcelain').toString().trim());
} catch (e) {
/* empty */
}

const version = dirty ? `${headRev}-dirty-${Date.now().toString()}` : headRev;

console.log(`SvelteKit reloading version: ${version}`);

/** @type {import('@sveltejs/kit').Config} */
const config = {
Expand All @@ -8,7 +24,10 @@ const config = {
preprocess: [vitePreprocess({})],

kit: {
adapter: adapter()
adapter: adapter(),
version: {
name: version
}
}
};

Expand Down
2 changes: 1 addition & 1 deletion frontend/tests/screencaps.spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -308,7 +308,7 @@ colorSchemes.forEach((colorScheme) => {
});

await expect(page).toHaveScreenshot({
maxDiffPixels: 2500
maxDiffPixels: 3000
});
});

Expand Down
Loading