From 0745faaee4b3219061b7d7eb8e071fcf46cf696f Mon Sep 17 00:00:00 2001 From: Devansh Singh Date: Mon, 17 Jun 2024 21:47:53 +0530 Subject: [PATCH 01/12] Add presets table and setup alembic for migrations Signed-off-by: Devansh Singh --- .env.dev | 2 + .gitignore | 3 + alembic.ini | 116 ++++++++++++++++++ migrations/README | 1 + migrations/env.py | 83 +++++++++++++ migrations/script.py.mako | 27 ++++ .../db0c0dc793f4_add_presets_table.py | 43 +++++++ setup.cfg | 2 + src/teuthology_api/models/__init__.py | 16 +++ src/teuthology_api/models/presets.py | 11 ++ 10 files changed, 304 insertions(+) create mode 100644 alembic.ini create mode 100644 migrations/README create mode 100644 migrations/env.py create mode 100644 migrations/script.py.mako create mode 100644 migrations/versions/db0c0dc793f4_add_presets_table.py create mode 100644 src/teuthology_api/models/__init__.py create mode 100644 src/teuthology_api/models/presets.py diff --git a/.env.dev b/.env.dev index d086a11..c998f35 100644 --- a/.env.dev +++ b/.env.dev @@ -17,3 +17,5 @@ SESSION_SECRET_KEY=my-secret-key # Path where all logs for teuthology-suite or teuthology-kill would be collected ARCHIVE_DIR=/archive_dir/ TEUTHOLOGY_PATH=/teuthology + +TEUTHOLOGY_API_SQLITE_URI= diff --git a/.gitignore b/.gitignore index 3d98dbd..ee326a9 100644 --- a/.gitignore +++ b/.gitignore @@ -53,3 +53,6 @@ MANIFEST .conda*/ .python-version venv + +# Environment Variables +.env diff --git a/alembic.ini b/alembic.ini new file mode 100644 index 0000000..f705021 --- /dev/null +++ b/alembic.ini @@ -0,0 +1,116 @@ +# A generic, single database configuration. + +[alembic] +# path to migration scripts +script_location = migrations + +# template used to generate migration file names; The default value is %%(rev)s_%%(slug)s +# Uncomment the line below if you want the files to be prepended with date and time +# see https://alembic.sqlalchemy.org/en/latest/tutorial.html#editing-the-ini-file +# for all available tokens +# file_template = %%(year)d_%%(month).2d_%%(day).2d_%%(hour).2d%%(minute).2d-%%(rev)s_%%(slug)s + +# sys.path path, will be prepended to sys.path if present. +# defaults to the current working directory. +prepend_sys_path = . + +# timezone to use when rendering the date within the migration file +# as well as the filename. +# If specified, requires the python>=3.9 or backports.zoneinfo library. +# Any required deps can installed by adding `alembic[tz]` to the pip requirements +# string value is passed to ZoneInfo() +# leave blank for localtime +# timezone = + +# max length of characters to apply to the +# "slug" field +# truncate_slug_length = 40 + +# set to 'true' to run the environment during +# the 'revision' command, regardless of autogenerate +# revision_environment = false + +# set to 'true' to allow .pyc and .pyo files without +# a source .py file to be detected as revisions in the +# versions/ directory +# sourceless = false + +# version location specification; This defaults +# to migrations/versions. When using multiple version +# directories, initial revisions must be specified with --version-path. +# The path separator used here should be the separator specified by "version_path_separator" below. +# version_locations = %(here)s/bar:%(here)s/bat:migrations/versions + +# version path separator; As mentioned above, this is the character used to split +# version_locations. The default within new alembic.ini files is "os", which uses os.pathsep. +# If this key is omitted entirely, it falls back to the legacy behavior of splitting on spaces and/or commas. +# Valid values for version_path_separator are: +# +# version_path_separator = : +# version_path_separator = ; +# version_path_separator = space +version_path_separator = os # Use os.pathsep. Default configuration used for new projects. + +# set to 'true' to search source files recursively +# in each "version_locations" directory +# new in Alembic version 1.10 +# recursive_version_locations = false + +# the output encoding used when revision files +# are written from script.py.mako +# output_encoding = utf-8 + +sqlalchemy.url = + + +[post_write_hooks] +# post_write_hooks defines scripts or Python functions that are run +# on newly generated revision scripts. See the documentation for further +# detail and examples + +# format using "black" - use the console_scripts runner, against the "black" entrypoint +# hooks = black +# black.type = console_scripts +# black.entrypoint = black +# black.options = -l 79 REVISION_SCRIPT_FILENAME + +# lint with attempts to fix using "ruff" - use the exec runner, execute a binary +# hooks = ruff +# ruff.type = exec +# ruff.executable = %(here)s/.venv/bin/ruff +# ruff.options = --fix REVISION_SCRIPT_FILENAME + +# Logging configuration +[loggers] +keys = root,sqlalchemy,alembic + +[handlers] +keys = console + +[formatters] +keys = generic + +[logger_root] +level = WARN +handlers = console +qualname = + +[logger_sqlalchemy] +level = WARN +handlers = +qualname = sqlalchemy.engine + +[logger_alembic] +level = INFO +handlers = +qualname = alembic + +[handler_console] +class = StreamHandler +args = (sys.stderr,) +level = NOTSET +formatter = generic + +[formatter_generic] +format = %(levelname)-5.5s [%(name)s] %(message)s +datefmt = %H:%M:%S diff --git a/migrations/README b/migrations/README new file mode 100644 index 0000000..98e4f9c --- /dev/null +++ b/migrations/README @@ -0,0 +1 @@ +Generic single-database configuration. \ No newline at end of file diff --git a/migrations/env.py b/migrations/env.py new file mode 100644 index 0000000..bd414bc --- /dev/null +++ b/migrations/env.py @@ -0,0 +1,83 @@ +from logging.config import fileConfig + +from src.teuthology_api.models import DATABASE_URL, Presets +from sqlalchemy import engine_from_config, pool +from sqlmodel import SQLModel + +from alembic import context + +# this is the Alembic Config object, which provides +# access to the values within the .ini file in use. +config = context.config +config.set_main_option("sqlalchemy.url", DATABASE_URL) + +# Interpret the config file for Python logging. +# This line sets up loggers basically. +if config.config_file_name is not None: + fileConfig(config.config_file_name) + +# add your model's MetaData object here +# for 'autogenerate' support +# from myapp import mymodel +# target_metadata = mymodel.Base.metadata +target_metadata = SQLModel.metadata + +# other values from the config, defined by the needs of env.py, +# can be acquired: +# my_important_option = config.get_main_option("my_important_option") +# ... etc. + + +def run_migrations_offline() -> None: + """Run migrations in 'offline' mode. + + This configures the context with just a URL + and not an Engine, though an Engine is acceptable + here as well. By skipping the Engine creation + we don't even need a DBAPI to be available. + + Calls to context.execute() here emit the given string to the + script output. + + """ + url = config.get_main_option("sqlalchemy.url") + context.configure( + url=url, + target_metadata=target_metadata, + literal_binds=True, + dialect_opts={"paramstyle": "named"}, + render_as_batch=True, + ) + + with context.begin_transaction(): + context.run_migrations() + + +def run_migrations_online() -> None: + """Run migrations in 'online' mode. + + In this scenario we need to create an Engine + and associate a connection with the context. + + """ + connectable = engine_from_config( + config.get_section(config.config_ini_section, {}), + prefix="sqlalchemy.", + poolclass=pool.NullPool, + ) + + with connectable.connect() as connection: + context.configure( + connection=connection, + target_metadata=target_metadata, + render_as_batch=True, + ) + + with context.begin_transaction(): + context.run_migrations() + + +if context.is_offline_mode(): + run_migrations_offline() +else: + run_migrations_online() diff --git a/migrations/script.py.mako b/migrations/script.py.mako new file mode 100644 index 0000000..6ce3351 --- /dev/null +++ b/migrations/script.py.mako @@ -0,0 +1,27 @@ +"""${message} + +Revision ID: ${up_revision} +Revises: ${down_revision | comma,n} +Create Date: ${create_date} + +""" +from typing import Sequence, Union + +from alembic import op +import sqlalchemy as sa +import sqlmodel +${imports if imports else ""} + +# revision identifiers, used by Alembic. +revision: str = ${repr(up_revision)} +down_revision: Union[str, None] = ${repr(down_revision)} +branch_labels: Union[str, Sequence[str], None] = ${repr(branch_labels)} +depends_on: Union[str, Sequence[str], None] = ${repr(depends_on)} + + +def upgrade() -> None: + ${upgrades if upgrades else "pass"} + + +def downgrade() -> None: + ${downgrades if downgrades else "pass"} diff --git a/migrations/versions/db0c0dc793f4_add_presets_table.py b/migrations/versions/db0c0dc793f4_add_presets_table.py new file mode 100644 index 0000000..df60e7d --- /dev/null +++ b/migrations/versions/db0c0dc793f4_add_presets_table.py @@ -0,0 +1,43 @@ +"""Add presets table + +Revision ID: db0c0dc793f4 +Revises: +Create Date: 2024-06-17 21:44:58.730544 + +""" + +from typing import Sequence, Union + +from alembic import op +import sqlalchemy as sa +import sqlmodel + + +# revision identifiers, used by Alembic. +revision: str = "db0c0dc793f4" +down_revision: Union[str, None] = None +branch_labels: Union[str, Sequence[str], None] = None +depends_on: Union[str, Sequence[str], None] = None + + +def upgrade() -> None: + # ### commands auto generated by Alembic - please adjust! ### + op.create_table( + "presets", + sa.Column("id", sa.Integer(), nullable=False), + sa.Column("username", sqlmodel.sql.sqltypes.AutoString(), nullable=False), + sa.Column("name", sqlmodel.sql.sqltypes.AutoString(), nullable=False), + sa.Column("suite", sqlmodel.sql.sqltypes.AutoString(), nullable=False), + sa.Column("cmd", sqlmodel.sql.sqltypes.AutoString(), nullable=False), + sa.PrimaryKeyConstraint("id"), + sa.UniqueConstraint("username", "name"), + ) + op.create_index(op.f("ix_presets_username"), "presets", ["username"]) + # ### end Alembic commands ### + + +def downgrade() -> None: + # ### commands auto generated by Alembic - please adjust! ### + op.drop_index(op.f("ix_presets_username"), table_name="presets") + op.drop_table("presets") + # ### end Alembic commands ### diff --git a/setup.cfg b/setup.cfg index 1304513..e78f617 100644 --- a/setup.cfg +++ b/setup.cfg @@ -56,6 +56,8 @@ install_requires = itsdangerous python-dotenv teuthology @ git+https://github.com/ceph/teuthology#egg=teuthology[test] + sqlmodel + alembic [options.packages.find] diff --git a/src/teuthology_api/models/__init__.py b/src/teuthology_api/models/__init__.py new file mode 100644 index 0000000..a32f701 --- /dev/null +++ b/src/teuthology_api/models/__init__.py @@ -0,0 +1,16 @@ +import os +from dotenv import load_dotenv +from sqlmodel import create_engine, Session + +from teuthology_api.models.presets import Presets + +load_dotenv() + +DATABASE_URL = os.getenv("TEUTHOLOGY_API_SQLITE_URI") + +engine = create_engine(DATABASE_URL) + + +def get_db(): + with Session(engine) as session: + yield session diff --git a/src/teuthology_api/models/presets.py b/src/teuthology_api/models/presets.py new file mode 100644 index 0000000..cdd7f17 --- /dev/null +++ b/src/teuthology_api/models/presets.py @@ -0,0 +1,11 @@ +from sqlmodel import Field, SQLModel, UniqueConstraint + + +class Presets(SQLModel, table=True): + id: int = Field(primary_key=True) + username: str = Field(index=True) + name: str + suite: str + cmd: str + + __table_args__ = (UniqueConstraint("username", "name"),) From edd9da5fd5decd7752d1f8bead6d68f960a4c2c3 Mon Sep 17 00:00:00 2001 From: Devansh Singh Date: Mon, 17 Jun 2024 21:50:30 +0530 Subject: [PATCH 02/12] Add services/presets.py Create presets service for database operations Signed-off-by: Devansh Singh --- src/teuthology_api/services/presets.py | 57 ++++++++++++++++++++++++++ 1 file changed, 57 insertions(+) create mode 100644 src/teuthology_api/services/presets.py diff --git a/src/teuthology_api/services/presets.py b/src/teuthology_api/services/presets.py new file mode 100644 index 0000000..ab85639 --- /dev/null +++ b/src/teuthology_api/services/presets.py @@ -0,0 +1,57 @@ +from sqlmodel import select, Session + +from teuthology_api.models.presets import Presets + + +class PresetsDatabaseException(Exception): + def __init__(self, message: str, code: int) -> None: + super().__init__(message) + self.code = code + + +class PresetsService: + def __init__(self, db: Session) -> None: + self.db = db + + def get_by_username(self, username: str): + statement = select(Presets).where(Presets.username == username) + db_presets = self.db.exec(statement).all() + return db_presets + + def get_by_username_and_name(self, username: str, preset_name: str): + statement = select(Presets).where( + Presets.username == username, Presets.name == preset_name + ) + db_preset = self.db.exec(statement).first() + return db_preset + + def get_by_id(self, preset_id: int): + statement = select(Presets).where(Presets.id == preset_id) + db_preset = self.db.exec(statement).first() + return db_preset + + def create(self, preset: Presets) -> Presets: + self.db.add(preset) + self.db.commit() + self.db.refresh(preset) + return preset + + def update(self, preset_id: int, updated_data: dict) -> Presets: + db_preset = self.get_by_id(preset_id) + if db_preset is None: + raise PresetsDatabaseException( + "Preset does not exist, unable to update", 404 + ) + + db_preset.sqlmodel_update(updated_data) + return self.create(db_preset) + + def delete(self, preset_id: int): + db_preset = self.get_by_id(preset_id) + if db_preset is None: + raise PresetsDatabaseException( + "Preset does not exist, unable to delete", 404 + ) + + self.db.delete(db_preset) + self.db.commit() From 35838c77b2a4471d8bae1c69c45e85c979f2eaae Mon Sep 17 00:00:00 2001 From: Devansh Singh Date: Mon, 17 Jun 2024 21:59:24 +0530 Subject: [PATCH 03/12] Add routes/presets.py Create endpoints for presets Signed-off-by: Devansh Singh --- src/teuthology_api/main.py | 3 +- src/teuthology_api/routes/presets.py | 96 ++++++++++++++++++++++++++ src/teuthology_api/services/presets.py | 10 ++- start_container.sh | 2 + 4 files changed, 107 insertions(+), 4 deletions(-) create mode 100644 src/teuthology_api/routes/presets.py diff --git a/src/teuthology_api/main.py b/src/teuthology_api/main.py index c1d8afb..e65261c 100644 --- a/src/teuthology_api/main.py +++ b/src/teuthology_api/main.py @@ -5,7 +5,7 @@ from dotenv import load_dotenv from starlette.middleware.sessions import SessionMiddleware -from teuthology_api.routes import suite, kill, login, logout +from teuthology_api.routes import suite, kill, login, logout, presets load_dotenv() @@ -40,3 +40,4 @@ def read_root(request: Request): app.include_router(kill.router) app.include_router(login.router) app.include_router(logout.router) +app.include_router(presets.router) diff --git a/src/teuthology_api/routes/presets.py b/src/teuthology_api/routes/presets.py new file mode 100644 index 0000000..b6cb32e --- /dev/null +++ b/src/teuthology_api/routes/presets.py @@ -0,0 +1,96 @@ +import logging + +from fastapi import status, APIRouter, HTTPException, Depends +from sqlmodel import Session + +from teuthology_api.services.helpers import get_token +from teuthology_api.models import get_db, Presets +from teuthology_api.services.presets import PresetsDatabaseException, PresetsService + +logger = logging.getLogger(__name__) + +router = APIRouter(prefix="/presets", tags=["presets"]) + + +@router.get("/", status_code=status.HTTP_200_OK) +def read_preset(username: str, name: str, db: Session = Depends(get_db)): + db_preset = PresetsService(db).get_by_username_and_name(username, name) + if db_preset is None: + raise HTTPException( + status_code=status.HTTP_404_NOT_FOUND, + detail=f"{name} preset does not exist.", + ) + return db_preset + + +@router.get("/list", status_code=status.HTTP_200_OK) +def read_all_presets(username: str, db: Session = Depends(get_db)): + db_presets = PresetsService(db).get_by_username(username) + if not db_presets: + raise HTTPException( + status_code=status.HTTP_404_NOT_FOUND, + detail="User has not presets saved.", + ) + return db_presets + + +@router.post("/add", status_code=status.HTTP_201_CREATED) +def add_preset( + preset: Presets, + db: Session = Depends(get_db), + access_token: str = Depends(get_token), +): + if not access_token: + raise HTTPException( + status_code=status.HTTP_401_UNAUTHORIZED, + detail="You need to be logged in", + headers={"WWW-Authenticate": "Bearer"}, + ) + db_preset_exists = PresetsService(db).get_by_username_and_name( + preset.username, preset.name + ) + if db_preset_exists: + raise HTTPException( + status_code=status.HTTP_400_BAD_REQUEST, + detail=f"Preset with name {preset.name} exists", + ) + return PresetsService(db).create(preset) + + +@router.put("/edit/{preset_id}", status_code=status.HTTP_200_OK) +def update_preset( + preset_id: int, + updated_preset: Presets, + db: Session = Depends(get_db), + access_token: str = Depends(get_token), +): + if not access_token: + raise HTTPException( + status_code=status.HTTP_401_UNAUTHORIZED, + detail="You need to be logged in", + headers={"WWW-Authenticate": "Bearer"}, + ) + try: + return PresetsService(db).update( + preset_id, updated_preset.model_dump(exclude_unset=True) + ) + except PresetsDatabaseException as exc: + raise HTTPException(status_code=exc.code, detail=str(exc)) + + +@router.delete("/delete/{preset_id}", status_code=status.HTTP_204_NO_CONTENT) +def delete_preset( + preset_id: int, + db: Session = Depends(get_db), + access_token: str = Depends(get_token), +): + if not access_token: + raise HTTPException( + status_code=status.HTTP_401_UNAUTHORIZED, + detail="You need to be logged in", + headers={"WWW-Authenticate": "Bearer"}, + ) + try: + PresetsService(db).delete(preset_id) + except PresetsDatabaseException as exc: + raise HTTPException(status_code=exc.code, detail=str(exc)) diff --git a/src/teuthology_api/services/presets.py b/src/teuthology_api/services/presets.py index ab85639..5136ea6 100644 --- a/src/teuthology_api/services/presets.py +++ b/src/teuthology_api/services/presets.py @@ -1,3 +1,5 @@ +from typing import Union + from sqlmodel import select, Session from teuthology_api.models.presets import Presets @@ -18,14 +20,16 @@ def get_by_username(self, username: str): db_presets = self.db.exec(statement).all() return db_presets - def get_by_username_and_name(self, username: str, preset_name: str): + def get_by_username_and_name( + self, username: str, preset_name: str + ) -> Union[Presets, None]: statement = select(Presets).where( Presets.username == username, Presets.name == preset_name ) db_preset = self.db.exec(statement).first() return db_preset - def get_by_id(self, preset_id: int): + def get_by_id(self, preset_id: int) -> Union[Presets, None]: statement = select(Presets).where(Presets.id == preset_id) db_preset = self.db.exec(statement).first() return db_preset @@ -46,7 +50,7 @@ def update(self, preset_id: int, updated_data: dict) -> Presets: db_preset.sqlmodel_update(updated_data) return self.create(db_preset) - def delete(self, preset_id: int): + def delete(self, preset_id: int) -> None: db_preset = self.get_by_id(preset_id) if db_preset is None: raise PresetsDatabaseException( diff --git a/start_container.sh b/start_container.sh index 2d9919c..7f6af77 100755 --- a/start_container.sh +++ b/start_container.sh @@ -9,6 +9,8 @@ VENV=${VENV:-"venv"} source ${VENV}/bin/activate cd /teuthology_api/src/ +alembic -x verbose=1 upgrade head + if [ "$DEPLOYMENT" = "development" ]; then uvicorn teuthology_api.main:app --reload --port $PORT --host $HOST else From 3d3f4b52a010407ab1576c0d0eaaff6df42df42a Mon Sep 17 00:00:00 2001 From: Devansh Singh Date: Mon, 1 Jul 2024 20:12:00 +0530 Subject: [PATCH 04/12] tests/conftest.py: Add fixtures Add fixtures to conftest.py for database session and FastAPI TestClient Signed-off-by: Devansh Singh --- tests/conftest.py | 39 ++++++++++++++++++++++++++++++++++++++- tests/test_helpers.py | 6 +----- tests/test_kill.py | 18 ++++-------------- tests/test_suite.py | 18 +++++------------- 4 files changed, 48 insertions(+), 33 deletions(-) diff --git a/tests/conftest.py b/tests/conftest.py index 8f9e147..326dd05 100644 --- a/tests/conftest.py +++ b/tests/conftest.py @@ -7,4 +7,41 @@ - https://docs.pytest.org/en/stable/writing_plugins.html """ -# import pytest +import pytest + +from fastapi.testclient import TestClient +from sqlmodel import Session, SQLModel, create_engine +from sqlmodel.pool import StaticPool + +from teuthology_api.main import app +from teuthology_api.models import get_db +from teuthology_api.services.helpers import get_token + + +def override_get_token(): + return {"access_token": "token_123", "token_type": "bearer"} + + +@pytest.fixture(name="session", scope="session") +def session_fixture(): + engine = create_engine( + "sqlite://", + connect_args={"check_same_thread": False}, + poolclass=StaticPool, + ) + SQLModel.metadata.create_all(engine) + with Session(engine) as session: + yield session + + +@pytest.fixture(name="client", scope="session") +def client_fixture(session: Session): + def override_get_db(): + return session + + app.dependency_overrides[get_db] = override_get_db + app.dependency_overrides[get_token] = override_get_token + + client = TestClient(app) + yield client + app.dependency_overrides.clear() diff --git a/tests/test_helpers.py b/tests/test_helpers.py index cda9a64..dad59d7 100644 --- a/tests/test_helpers.py +++ b/tests/test_helpers.py @@ -1,11 +1,7 @@ -from fastapi.testclient import TestClient from fastapi import HTTPException import pytest -from teuthology_api.main import app from unittest.mock import patch -from teuthology_api.services.helpers import Request, get_token, get_username - -client = TestClient(app) +from teuthology_api.services.helpers import get_token, get_username class MockRequest: diff --git a/tests/test_kill.py b/tests/test_kill.py index 7a30953..9de696b 100644 --- a/tests/test_kill.py +++ b/tests/test_kill.py @@ -1,19 +1,7 @@ from fastapi.testclient import TestClient -from teuthology_api.main import app from unittest.mock import patch -from teuthology_api.services.helpers import get_token -from teuthology_api.services.kill import get_username, get_run_details import json -from teuthology_api.schemas.kill import KillArgs -client = TestClient(app) - - -async def override_get_token(): - return {"access_token": "token_123", "token_type": "bearer"} - - -app.dependency_overrides[get_token] = override_get_token mock_kill_args = { "--dry-run": False, @@ -33,7 +21,9 @@ async def override_get_token(): @patch("subprocess.Popen") @patch("teuthology_api.services.kill.get_run_details") @patch("teuthology_api.services.kill.get_username") -def test_kill_run_success(m_get_username, m_get_run_details, m_popen): +def test_kill_run_success( + m_get_username, m_get_run_details, m_popen, client: TestClient +): m_get_username.return_value = "user1" m_get_run_details.return_value = {"id": "7451978", "user": "user1"} mock_process = m_popen.return_value @@ -44,7 +34,7 @@ def test_kill_run_success(m_get_username, m_get_run_details, m_popen): assert response.json() == {"kill": "success"} -def test_kill_run_fail(): +def test_kill_run_fail(client: TestClient): response = client.post("/kill", data=json.dumps(mock_kill_args)) assert response.status_code == 401 assert response.json() == {"detail": "You need to be logged in"} diff --git a/tests/test_suite.py b/tests/test_suite.py index ab2f6f7..3117bc9 100644 --- a/tests/test_suite.py +++ b/tests/test_suite.py @@ -1,19 +1,8 @@ from fastapi.testclient import TestClient -from teuthology_api.main import app from unittest.mock import patch -from teuthology_api.services.helpers import get_token -from teuthology_api.services.suite import make_run_name, get_run_details +from teuthology_api.services.suite import make_run_name import json -client = TestClient(app) - - -async def override_get_token(): - return {"access_token": "token_123", "token_type": "bearer"} - - -app.dependency_overrides[get_token] = override_get_token - mock_suite_args = { "--dry-run": False, "--non-interactive": False, @@ -28,11 +17,14 @@ async def override_get_token(): "--machine-type": "testnode", } + # suite @patch("teuthology_api.services.suite.logs_run") @patch("teuthology_api.routes.suite.get_username") @patch("teuthology_api.services.suite.get_run_details") -def test_suite_run_success(m_get_run_details, m_get_username, m_logs_run): +def test_suite_run_success( + m_get_run_details, m_get_username, m_logs_run, client: TestClient +): m_get_username.return_value = "user1" m_get_run_details.return_value = {"id": "7451978", "user": "user1"} response = client.post("/suite", data=json.dumps(mock_suite_args)) From b78886aa2867458d434dcfcb079b2c381c24d9ee Mon Sep 17 00:00:00 2001 From: Devansh Singh Date: Mon, 1 Jul 2024 23:15:09 +0530 Subject: [PATCH 05/12] Add tests/test_preset.py Add unittests for /presets endpoint Signed-off-by: Devansh Singh --- tests/test_preset.py | 50 ++++++++++++++++++++++++++++++++++++++++++++ 1 file changed, 50 insertions(+) create mode 100644 tests/test_preset.py diff --git a/tests/test_preset.py b/tests/test_preset.py new file mode 100644 index 0000000..11efcc8 --- /dev/null +++ b/tests/test_preset.py @@ -0,0 +1,50 @@ +from urllib.parse import urlencode + +from fastapi.testclient import TestClient +from sqlmodel import Session + +preset_payload = { + "username": "user1", + "name": "test", + "suite": "teuthology:no-ceph", + "cmd": '{"owner": "user1"}', +} + + +def test_create_preset(session: Session, client: TestClient): + response = client.post("/presets/add", json=preset_payload) + assert response.status_code == 201 + + +def test_get_preset_by_name(session: Session, client: TestClient): + params = {"username": "user1", "name": "test"} + response = client.get(f"/presets?{urlencode(params)}") + data = response.json() + + assert response.status_code == 200 + assert data["username"] == "user1" + assert data["name"] == "test" + + +def test_get_all_presets(session: Session, client: TestClient): + params = {"username": "user1"} + response = client.get(f"/presets/list?{urlencode(params)}") + data = response.json() + + assert response.status_code == 200 + assert len(data) == 1 + + +def test_update_preset(session: Session, client: TestClient): + preset_payload["name"] = "test-updated" + response = client.put("/presets/edit/1", json=preset_payload) + data = response.json() + + assert response.status_code == 200 + assert data["username"] == "user1" + assert data["name"] == "test-updated" + + +def test_delete_preset(session: Session, client: TestClient): + response = client.delete("/presets/delete/1") + assert response.status_code == 204 From b2a7de88bc0802efaf22b55f426a2a73857177b7 Mon Sep 17 00:00:00 2001 From: Devansh Singh Date: Tue, 2 Jul 2024 20:40:04 +0530 Subject: [PATCH 06/12] gh-actions/start.sh: Add database environment variable Signed-off-by: Devansh Singh --- gh-actions/start.sh | 3 ++- start_container.sh | 4 ++-- 2 files changed, 4 insertions(+), 3 deletions(-) diff --git a/gh-actions/start.sh b/gh-actions/start.sh index 7bd37e1..1425934 100755 --- a/gh-actions/start.sh +++ b/gh-actions/start.sh @@ -8,9 +8,10 @@ if [ ! -d "$folder" ] ; then context: ../../../../ ports: - 8082:8080 - environment: + environment: TEUTHOLOGY_API_SERVER_HOST: 0.0.0.0 TEUTHOLOGY_API_SERVER_PORT: 8080 + TEUTHOLOGY_API_SQLITE_URI: sqlite:////teuthology_api/teuthology.db depends_on: - teuthology - paddles diff --git a/start_container.sh b/start_container.sh index 7f6af77..6628cf0 100755 --- a/start_container.sh +++ b/start_container.sh @@ -7,10 +7,10 @@ PORT=${TEUTHOLOGY_API_SERVER_PORT:-"8082"} VENV=${VENV:-"venv"} source ${VENV}/bin/activate -cd /teuthology_api/src/ - alembic -x verbose=1 upgrade head +cd /teuthology_api/src/ + if [ "$DEPLOYMENT" = "development" ]; then uvicorn teuthology_api.main:app --reload --port $PORT --host $HOST else From 54099fc893533a3f5dffbaf350b1a3e2b04e0d84 Mon Sep 17 00:00:00 2001 From: Devansh Singh Date: Wed, 3 Jul 2024 21:47:45 +0530 Subject: [PATCH 07/12] .github/workflows/unit_tests.yaml: Add database env variable Add TEUTHOLOGY_API_SQLITE_URI environment variable to tox.ini and unit_tests.yaml Signed-off-by: Devansh Singh --- .github/workflows/unit_tests.yaml | 2 ++ tox.ini | 1 + 2 files changed, 3 insertions(+) diff --git a/.github/workflows/unit_tests.yaml b/.github/workflows/unit_tests.yaml index f0e1f1a..96d8bbc 100644 --- a/.github/workflows/unit_tests.yaml +++ b/.github/workflows/unit_tests.yaml @@ -3,6 +3,8 @@ on: pull_request jobs: build: + env: + TEUTHOLOGY_API_SQLITE_URI: ${{ secrets.TEUTHOLOGY_API_SQLITE_URI }} name: Unit Tests on python${{ matrix.python }} via ${{ matrix.os }} runs-on: ${{ matrix.os }} strategy: diff --git a/tox.ini b/tox.ini index 69f8159..b0e5822 100644 --- a/tox.ini +++ b/tox.ini @@ -15,6 +15,7 @@ setenv = passenv = HOME SETUPTOOLS_* + TEUTHOLOGY_API_SQLITE_URI extras = testing commands = From bd3b11db598179e8eb349af45a92c26a87e41662 Mon Sep 17 00:00:00 2001 From: Devansh Singh Date: Thu, 1 Aug 2024 21:04:17 +0530 Subject: [PATCH 08/12] Add check for preset limit Added a limit on how many presets a user can store at a time (currently 10) Signed-off-by: Devansh Singh --- src/teuthology_api/routes/presets.py | 13 +++++++++++++ src/teuthology_api/services/presets.py | 4 +++- 2 files changed, 16 insertions(+), 1 deletion(-) diff --git a/src/teuthology_api/routes/presets.py b/src/teuthology_api/routes/presets.py index b6cb32e..0087aad 100644 --- a/src/teuthology_api/routes/presets.py +++ b/src/teuthology_api/routes/presets.py @@ -37,6 +37,7 @@ def read_all_presets(username: str, db: Session = Depends(get_db)): @router.post("/add", status_code=status.HTTP_201_CREATED) def add_preset( preset: Presets, + replace: bool = False, db: Session = Depends(get_db), access_token: str = Depends(get_token), ): @@ -46,6 +47,18 @@ def add_preset( detail="You need to be logged in", headers={"WWW-Authenticate": "Bearer"}, ) + + db_presets = PresetsService(db).get_by_username(preset.username) + if len(db_presets) == 10: + if not replace: + raise HTTPException( + status_code=status.HTTP_400_BAD_REQUEST, + detail="Only 10 presets for a user can be stored at a time.", + ) + # If replace parameter is set true, delete the + # oldest preset of the user and create a new one + PresetsService(db).delete(db_presets[0].id) + db_preset_exists = PresetsService(db).get_by_username_and_name( preset.username, preset.name ) diff --git a/src/teuthology_api/services/presets.py b/src/teuthology_api/services/presets.py index 5136ea6..9571735 100644 --- a/src/teuthology_api/services/presets.py +++ b/src/teuthology_api/services/presets.py @@ -16,7 +16,9 @@ def __init__(self, db: Session) -> None: self.db = db def get_by_username(self, username: str): - statement = select(Presets).where(Presets.username == username) + statement = ( + select(Presets).where(Presets.username == username).order_by(Presets.id) + ) db_presets = self.db.exec(statement).all() return db_presets From 955c1dcb4fcf8e02d7a0c27cc8ed6326ef0743db Mon Sep 17 00:00:00 2001 From: Devansh Singh Date: Tue, 13 Aug 2024 18:52:00 +0530 Subject: [PATCH 09/12] Add PresetArgs schema, update Presets cmd column type Added a schema for /add endpoint request body for validating cmd attribute. Updated the cmd column datatype from string to JSON for the Presets model. Signed-off-by: Devansh Singh --- .../db0c0dc793f4_add_presets_table.py | 2 +- src/teuthology_api/models/presets.py | 7 +++++-- src/teuthology_api/routes/presets.py | 19 ++++++++++++------- src/teuthology_api/schemas/preset.py | 12 ++++++++++++ tests/test_preset.py | 11 ++++++++--- 5 files changed, 38 insertions(+), 13 deletions(-) create mode 100644 src/teuthology_api/schemas/preset.py diff --git a/migrations/versions/db0c0dc793f4_add_presets_table.py b/migrations/versions/db0c0dc793f4_add_presets_table.py index df60e7d..26c73e9 100644 --- a/migrations/versions/db0c0dc793f4_add_presets_table.py +++ b/migrations/versions/db0c0dc793f4_add_presets_table.py @@ -28,7 +28,7 @@ def upgrade() -> None: sa.Column("username", sqlmodel.sql.sqltypes.AutoString(), nullable=False), sa.Column("name", sqlmodel.sql.sqltypes.AutoString(), nullable=False), sa.Column("suite", sqlmodel.sql.sqltypes.AutoString(), nullable=False), - sa.Column("cmd", sqlmodel.sql.sqltypes.AutoString(), nullable=False), + sa.Column("cmd", sa.JSON(), nullable=False), sa.PrimaryKeyConstraint("id"), sa.UniqueConstraint("username", "name"), ) diff --git a/src/teuthology_api/models/presets.py b/src/teuthology_api/models/presets.py index cdd7f17..7a3aebb 100644 --- a/src/teuthology_api/models/presets.py +++ b/src/teuthology_api/models/presets.py @@ -1,11 +1,14 @@ -from sqlmodel import Field, SQLModel, UniqueConstraint +from pydantic import ConfigDict +from sqlmodel import Column, Field, JSON, SQLModel, UniqueConstraint class Presets(SQLModel, table=True): + model_config = ConfigDict(arbitrary_types_allowed=True) + id: int = Field(primary_key=True) username: str = Field(index=True) name: str suite: str - cmd: str + cmd: JSON = Field(sa_column=Column(JSON)) __table_args__ = (UniqueConstraint("username", "name"),) diff --git a/src/teuthology_api/routes/presets.py b/src/teuthology_api/routes/presets.py index 0087aad..0e708c4 100644 --- a/src/teuthology_api/routes/presets.py +++ b/src/teuthology_api/routes/presets.py @@ -1,10 +1,11 @@ import logging -from fastapi import status, APIRouter, HTTPException, Depends +from fastapi import status, APIRouter, Depends, HTTPException, Request from sqlmodel import Session -from teuthology_api.services.helpers import get_token from teuthology_api.models import get_db, Presets +from teuthology_api.schemas.preset import PresetArgs +from teuthology_api.services.helpers import get_token, get_username from teuthology_api.services.presets import PresetsDatabaseException, PresetsService logger = logging.getLogger(__name__) @@ -36,7 +37,8 @@ def read_all_presets(username: str, db: Session = Depends(get_db)): @router.post("/add", status_code=status.HTTP_201_CREATED) def add_preset( - preset: Presets, + request: Request, + preset: PresetArgs, replace: bool = False, db: Session = Depends(get_db), access_token: str = Depends(get_token), @@ -48,7 +50,8 @@ def add_preset( headers={"WWW-Authenticate": "Bearer"}, ) - db_presets = PresetsService(db).get_by_username(preset.username) + username = get_username(request) + db_presets = PresetsService(db).get_by_username(username) if len(db_presets) == 10: if not replace: raise HTTPException( @@ -60,20 +63,22 @@ def add_preset( PresetsService(db).delete(db_presets[0].id) db_preset_exists = PresetsService(db).get_by_username_and_name( - preset.username, preset.name + username, preset.name ) if db_preset_exists: raise HTTPException( status_code=status.HTTP_400_BAD_REQUEST, detail=f"Preset with name {preset.name} exists", ) - return PresetsService(db).create(preset) + + db_preset = Presets(**preset.model_dump(), username=username) + return PresetsService(db).create(db_preset) @router.put("/edit/{preset_id}", status_code=status.HTTP_200_OK) def update_preset( preset_id: int, - updated_preset: Presets, + updated_preset: PresetArgs, db: Session = Depends(get_db), access_token: str = Depends(get_token), ): diff --git a/src/teuthology_api/schemas/preset.py b/src/teuthology_api/schemas/preset.py new file mode 100644 index 0000000..23fe19b --- /dev/null +++ b/src/teuthology_api/schemas/preset.py @@ -0,0 +1,12 @@ +from pydantic import BaseModel + +from teuthology_api.schemas.suite import SuiteArgs + + +class PresetArgs(BaseModel): + name: str + suite: str + cmd: SuiteArgs + + def model_post_init(self, __context): + self.cmd = self.cmd.model_dump() diff --git a/tests/test_preset.py b/tests/test_preset.py index 11efcc8..33d789c 100644 --- a/tests/test_preset.py +++ b/tests/test_preset.py @@ -1,17 +1,22 @@ +from unittest.mock import patch from urllib.parse import urlencode from fastapi.testclient import TestClient from sqlmodel import Session preset_payload = { - "username": "user1", "name": "test", "suite": "teuthology:no-ceph", - "cmd": '{"owner": "user1"}', + "cmd": { + "--suite": "teuthology:no-ceph", + "--owner": "user1", + }, } -def test_create_preset(session: Session, client: TestClient): +@patch("teuthology_api.routes.presets.get_username") +def test_create_preset(m_get_username, session: Session, client: TestClient): + m_get_username.return_value = "user1" response = client.post("/presets/add", json=preset_payload) assert response.status_code == 201 From 2b709625bf9785584c0fc7e07d0db1f3792771b1 Mon Sep 17 00:00:00 2001 From: Devansh Singh Date: Mon, 19 Aug 2024 19:02:38 +0530 Subject: [PATCH 10/12] Update documentation Signed-off-by: Devansh Singh --- README.md | 10 ++++++---- 1 file changed, 6 insertions(+), 4 deletions(-) diff --git a/README.md b/README.md index 8203a02..38b23e5 100644 --- a/README.md +++ b/README.md @@ -16,6 +16,8 @@ A REST API to execute [teuthology commands](https://docs.ceph.com/projects/teuth 3.3. Save `CLIENT_ID` and `CLIENT_SECRET` from your Github OAuth App to your local `.env` file as `GH_CLIENT_ID` and `GH_CLIENT_SECRET`. + 3.4. Add `TEUTHOLOGY_API_SQLITE_URI` using the format `sqlite:////` + 4. Add the following to [teuthology's docker-compose](https://github.com/ceph/teuthology/blob/main/docs/docker-compose/docker-compose.yml) services. ``` @@ -65,7 +67,9 @@ A REST API to execute [teuthology commands](https://docs.ceph.com/projects/teuth 6. Build the project: `pip install -e .` -7. Start the server: `gunicorn -c gunicorn_config.py teuthology_api.main:app` +7. Run `alembic upgrade head` command to run database migrations + +8. Start the server: `gunicorn -c gunicorn_config.py teuthology_api.main:app` ## Documentation @@ -108,6 +112,4 @@ Example "--owner": "example" }' -Note: "--owner" in data body should be same as your github username (case sensitive). Otherwise, you wouldn't have permission to kill jobs/run. - -xxx +> Note: `"--owner"` in data body should be same as your github username (case sensitive). Otherwise, you wouldn't have permission to kill jobs/run. From 72c88d9884ee0fc1d63763404607e70f116a58bb Mon Sep 17 00:00:00 2001 From: Devansh Singh Date: Wed, 21 Aug 2024 19:33:09 +0530 Subject: [PATCH 11/12] Add query parameter to search preset by suite Signed-off-by: Devansh Singh --- src/teuthology_api/routes/presets.py | 10 ++++++++-- src/teuthology_api/services/presets.py | 7 +++++++ 2 files changed, 15 insertions(+), 2 deletions(-) diff --git a/src/teuthology_api/routes/presets.py b/src/teuthology_api/routes/presets.py index 0e708c4..9cb8578 100644 --- a/src/teuthology_api/routes/presets.py +++ b/src/teuthology_api/routes/presets.py @@ -1,4 +1,5 @@ import logging +from typing import Optional from fastapi import status, APIRouter, Depends, HTTPException, Request from sqlmodel import Session @@ -25,8 +26,13 @@ def read_preset(username: str, name: str, db: Session = Depends(get_db)): @router.get("/list", status_code=status.HTTP_200_OK) -def read_all_presets(username: str, db: Session = Depends(get_db)): - db_presets = PresetsService(db).get_by_username(username) +def read_all_presets( + username: str, suite: Optional[str] = None, db: Session = Depends(get_db) +): + if suite: + db_presets = PresetsService(db).get_by_username_and_suite(username, suite) + else: + db_presets = PresetsService(db).get_by_username(username) if not db_presets: raise HTTPException( status_code=status.HTTP_404_NOT_FOUND, diff --git a/src/teuthology_api/services/presets.py b/src/teuthology_api/services/presets.py index 9571735..c1353a1 100644 --- a/src/teuthology_api/services/presets.py +++ b/src/teuthology_api/services/presets.py @@ -22,6 +22,13 @@ def get_by_username(self, username: str): db_presets = self.db.exec(statement).all() return db_presets + def get_by_username_and_suite(self, username: str, suite: str): + statement = select(Presets).where( + Presets.username == username, Presets.suite == suite + ) + db_presets = self.db.exec(statement).all() + return db_presets + def get_by_username_and_name( self, username: str, preset_name: str ) -> Union[Presets, None]: From 1b9424f585267e26ac81a0a6d1c1be6d3f419277 Mon Sep 17 00:00:00 2001 From: Devansh Singh Date: Mon, 2 Sep 2024 11:55:09 +0530 Subject: [PATCH 12/12] Convert username to lowercase, update request models for presets Removed the suite attribute from PresetArgs, created a new model for /update with optional preset fields Signed-off-by: Devansh Singh --- src/teuthology_api/routes/presets.py | 25 ++++++++++++++++++------- src/teuthology_api/schemas/preset.py | 14 ++++++++++++-- 2 files changed, 30 insertions(+), 9 deletions(-) diff --git a/src/teuthology_api/routes/presets.py b/src/teuthology_api/routes/presets.py index 9cb8578..03ab7e3 100644 --- a/src/teuthology_api/routes/presets.py +++ b/src/teuthology_api/routes/presets.py @@ -5,7 +5,7 @@ from sqlmodel import Session from teuthology_api.models import get_db, Presets -from teuthology_api.schemas.preset import PresetArgs +from teuthology_api.schemas.preset import PresetArgs, PresetUpdateArgs from teuthology_api.services.helpers import get_token, get_username from teuthology_api.services.presets import PresetsDatabaseException, PresetsService @@ -16,6 +16,8 @@ @router.get("/", status_code=status.HTTP_200_OK) def read_preset(username: str, name: str, db: Session = Depends(get_db)): + # GitHub usernames are case-insensitive + username = username.lower() db_preset = PresetsService(db).get_by_username_and_name(username, name) if db_preset is None: raise HTTPException( @@ -29,6 +31,7 @@ def read_preset(username: str, name: str, db: Session = Depends(get_db)): def read_all_presets( username: str, suite: Optional[str] = None, db: Session = Depends(get_db) ): + username = username.lower() if suite: db_presets = PresetsService(db).get_by_username_and_suite(username, suite) else: @@ -56,7 +59,7 @@ def add_preset( headers={"WWW-Authenticate": "Bearer"}, ) - username = get_username(request) + username = get_username(request).lower() db_presets = PresetsService(db).get_by_username(username) if len(db_presets) == 10: if not replace: @@ -77,14 +80,16 @@ def add_preset( detail=f"Preset with name {preset.name} exists", ) - db_preset = Presets(**preset.model_dump(), username=username) + db_preset = Presets( + **preset.model_dump(), username=username, suite=preset.cmd["--suite"] + ) return PresetsService(db).create(db_preset) @router.put("/edit/{preset_id}", status_code=status.HTTP_200_OK) def update_preset( preset_id: int, - updated_preset: PresetArgs, + preset: PresetUpdateArgs, db: Session = Depends(get_db), access_token: str = Depends(get_token), ): @@ -95,9 +100,15 @@ def update_preset( headers={"WWW-Authenticate": "Bearer"}, ) try: - return PresetsService(db).update( - preset_id, updated_preset.model_dump(exclude_unset=True) - ) + updated_preset = preset.model_dump(exclude_unset=True) + if updated_preset == {}: + raise HTTPException( + status_code=status.HTTP_400_BAD_REQUEST, + detail="Nothing new to update", + ) + if preset.cmd: + updated_preset["suite"] = preset.cmd["--suite"] + return PresetsService(db).update(preset_id, updated_preset) except PresetsDatabaseException as exc: raise HTTPException(status_code=exc.code, detail=str(exc)) diff --git a/src/teuthology_api/schemas/preset.py b/src/teuthology_api/schemas/preset.py index 23fe19b..6fdf304 100644 --- a/src/teuthology_api/schemas/preset.py +++ b/src/teuthology_api/schemas/preset.py @@ -1,3 +1,5 @@ +from typing import Optional + from pydantic import BaseModel from teuthology_api.schemas.suite import SuiteArgs @@ -5,8 +7,16 @@ class PresetArgs(BaseModel): name: str - suite: str cmd: SuiteArgs def model_post_init(self, __context): - self.cmd = self.cmd.model_dump() + self.cmd = self.cmd.model_dump(by_alias=True, exclude_unset=True) + + +class PresetUpdateArgs(BaseModel): + name: Optional[str] = None + cmd: Optional[SuiteArgs] = None + + def model_post_init(self, __context): + if self.cmd: + self.cmd = self.cmd.model_dump(by_alias=True, exclude_unset=True)