Fastapi #94

sfoale · 2025-06-09T15:16:22Z

Summary

• Implement complete FastAPI backend to replace legacy Flask application with modern async architecture
• Add comprehensive test coverage for all FastAPI endpoints with 11 test modules covering new code

Key Changes

New FastAPI Backend (server/)

Modern async FastAPI application with auto-generated OpenAPI documentation
SQLAlchemy 2.0+ models with PostGIS geospatial support
Pydantic schemas for request/response validation
Modular route structure organized by domain (pointing, instrument, alerts, etc.)
JWT authentication system replacing Flask-based auth

Comprehensive Test Suite (tests/fastapi/)

100% endpoint coverage across all API routes
Integration tests with database restore functionality
Test data fixtures for realistic testing scenarios

Kubernetes Integration (gwtm-helm/)

FastAPI deployment templates and configuration
Database initialization scripts for containerized environments
Development tooling with Skaffold for full-stack testing

This FastAPI implementation aims to maintain full API compatibility while providing better performance, automatic documentation, and type safety.

Refactor routes. Add README.

Fingel

I spent some time trying to get this running locally, but was ultimately unsuccessful.

I'm not clear on how the database is managed. There are some manual commands in the file database_postgresql_notes but if that is the de-facto method of getting a database set up, shouldn't it be a .sql file?

There seemed to be some syntax errors in there anyway, ($$to add enum type -> alter type bandpass add value 'other';) so even after copying the commands to a .sql file I was unable to use them to set up the database properly.

Without the database, it seems the tests will not run. Additionally, the tests appear to make HTTP requests to 2 different services. I managed to figure that one of them running on port 8000 was the fastapi service itself. But I don't know what the requests going to port 5000 are supposed to test.

All in all, I think the README needs some help.

Fingel · 2025-06-11T16:45:24Z

requirements.txt

 Flask-Mail==0.10.0
 Flask-SQLAlchemy==2.5.1
 Flask-WTF
+flask-caching


Missing fastapi

… application.

sfoale · 2025-06-12T15:10:07Z

I spent some time trying to get this running locally, but was ultimately unsuccessful.

I'm not clear on how the database is managed. There are some manual commands in the file database_postgresql_notes but if that is the de-facto method of getting a database set up, shouldn't it be a .sql file?

There seemed to be some syntax errors in there anyway, ($$to add enum type -> alter type bandpass add value 'other';) so even after copying the commands to a .sql file I was unable to use them to set up the database properly.

Without the database, it seems the tests will not run. Additionally, the tests appear to make HTTP requests to 2 different services. I managed to figure that one of them running on port 8000 was the fastapi service itself. But I don't know what the requests going to port 5000 are supposed to test.

All in all, I think the README needs some help.

Sorry about that - I've added a bit of signposting from the main readme to the fastapi readme.

With the project in flux with the intention to retire flask in favour of fastapi / svelte I wasn't concentrating so much on the top level README.

In summary, the most straightforward way to get the whole thing up and running - fastapi, database, even the flask app, is to use skaffold - just as
skaffold dev
should bring everything up. Then the tests can be run with a script (provided) - more info in the server/README.

Hope that is a bit clearer now.

Copilot

Pull Request Overview

This PR replaces the legacy Flask backend with a new async FastAPI application, adds Pydantic-based settings and JWT authentication, and integrates full Kubernetes/Helm/Skaffold deployment plus comprehensive endpoint tests.

Introduce a modern FastAPI service in server/ with SQLAlchemy models, Pydantic schemas, and JWT auth
Provide Pydantic settings in server/config.py and Dockerfile for containerization
Add Helm charts (gwtm-helm/) and Skaffold config for local and production deployments

Reviewed Changes

Copilot reviewed 147 out of 147 changed files in this pull request and generated 2 comments.

Show a summary per file

File	Description
server/core/enums/bandpass.py	New `bandpass` enum values
server/core/enums/init.py	Import list for core enums (duplicate import present)
server/config.py	Pydantic `Settings` class for environment variables
server/auth/auth.py	JWT token creation, decoding, and user validation
server/auth/init.py	Auth package init
server/README.md	FastAPI backend usage and testing instructions
server/Dockerfile	Container build for FastAPI service
gwtm-helm/values.yaml	Helm values for FastAPI deployment
gwtm-helm/values-dev.yaml	Development overrides for Helm
gwtm-helm/templates/ingress.yaml	Ingress paths for `/api/v1`, docs, health
gwtm-helm/templates/fastapi/service.yaml	Kubernetes Service definition for FastAPI
gwtm-helm/templates/fastapi/deployment.yaml	Kubernetes Deployment for FastAPI with init container
gwtm-helm/templates/fastapi/configmap.yaml	ConfigMap for FastAPI environment variables
gwtm-helm/templates/backend/create-database-tables.yaml	Database setup Job
gwtm-helm/start-dev.sh	Script to start Skaffold dev environment
gwtm-helm/skaffold.yaml	Skaffold configuration for both Flask and FastAPI
gwtm-helm/restore-db	Script for restoring database dumps
gwtm-helm/fastapi-README.md	Documentation for FastAPI Helm templates
README.md	Root README with FastAPI quick-start
.dockerignore	Updated ignore rules for Docker build

Comments suppressed due to low confidence (2)

server/core/enums/bandpass.py:3

Class names should use PascalCase. Rename bandpass to Bandpass to follow Python class naming conventions.

class bandpass(IntEnum):

gwtm-helm/templates/fastapi/configmap.yaml:13

CORS_ORIGINS is expected to be a list by the Pydantic settings. Providing a single string may cause parsing errors. Consider formatting it as a JSON array or comma-separated list that your app can split.

CORS_ORIGINS: "{{ .Values.ingress.host }}"

Copilot · 2025-06-16T19:20:57Z

server/core/enums/__init__.py

+from .pointing_status import pointing_status
+from .frequency_units import frequency_units
+from .depth_unit import depth_unit
+from .frequency_units import frequency_units


Duplicate import of frequency_units detected. Remove the redundant import to clean up the module.

Suggested change

from .frequency_units import frequency_units

Copilot · 2025-06-16T19:20:57Z

server/config.py

+    SECRET_KEY: str = Field(os.urandom(16).hex(), env="SECRET_KEY")
+    JWT_SECRET_KEY: str = Field(os.urandom(16).hex(), env="JWT_SECRET_KEY")


Using os.urandom at import time sets the same default each run and may not be re-evaluated per instance. Consider using default_factory=... to generate a new random key for each instantiation.

Suggested change

SECRET_KEY: str = Field(os.urandom(16).hex(), env="SECRET_KEY")

JWT_SECRET_KEY: str = Field(os.urandom(16).hex(), env="JWT_SECRET_KEY")

def _generate_random_key() -> str:

"""Generate a random 16-byte key and return its hexadecimal representation."""

return os.urandom(16).hex()

SECRET_KEY: str = Field(default_factory=_generate_random_key, env="SECRET_KEY")

JWT_SECRET_KEY: str = Field(default_factory=_generate_random_key, env="JWT_SECRET_KEY")

cmccully · 2025-07-02T15:54:23Z

server/core/enums/wavelengthunits.py

@@ -0,0 +1,17 @@
+from enum import IntEnum
+
+class WavelengthUnits(IntEnum):


Astropy has a pretty extensive units package developed. Does it help us to use that?

cmccully · 2025-07-02T15:56:35Z

server/core/enums/bandpass.py

@@ -0,0 +1,35 @@
+from enum import IntEnum
+
+class Bandpass(IntEnum):


This feels very fragile. Should this be more dynamic and stored in the db?

cmccully · 2026-02-10T20:47:22Z

server/utils/function.py

+        return False
+
+
+def isFloat(s) -> bool:


These actually check if things are float. They only test if something can be cast to a float. We should probably at least change the name to better reflect that. Aslo python doesn't use camel case methods mostly. These functions feel like overkill? Is it not better to just have a try except when we are casting? This feels kind of non-Pythonic (read Javaesque). This kind of feels like trying to get around duck typing.

cmccully · 2026-02-10T20:48:12Z

server/utils/function.py

+    Returns:
+        Projected footprint as a list of (ra, dec) tuples
+    """
+    # Calculate center of the footprint


Should we use either the astropy wcs or reproject package to do these checks so we don't have to maintain this?

cmccully · 2026-02-10T20:49:28Z

server/utils/function.py

+    chunks = []
+    for i in range(0, len(items), n):
+        chunks.append(items[i : i + n])
+    return chunks


Do we really need this function? It looks like we can just do this with index magic.

cmccully · 2026-02-10T20:50:02Z

server/utils/function.py

+        try:
+            return float(i)
+        except:  # noqa: E722
+            return 0.0


This scares me because 0.0 is number.

cmccully · 2026-02-10T20:50:29Z

server/utils/function.py

+    Returns:
+        Tuple of (doi_id, doi_url)
+    """
+    import uuid


Imports inside a method.

cmccully · 2026-02-10T20:51:01Z

server/utils/gwtm_io.py

+        config: Configuration object with credentials
+
+    Returns:
+        Filesystem object


Do we want to add support for local filesystems for development?

cmccully · 2026-02-10T20:52:55Z

server/utils/spectral.py

+            central_wave, bandwidth, bandpass_enum
+        )
+
+        freq_max = 2997924580000000000.0 / wave_min


I do not like these hardecoded numbers with lots of zeros Sam I Am. Can we use astropy constants?

cmccully · 2026-02-10T20:53:28Z

server/utils/spectral.py

+    return SPEED_OF_LIGHT / wavelength_m
+
+
+def freqToWave(freq: float) -> float:


It feels like a lot of these can be replaced with astropy unit conversions.

cmccully · 2026-02-10T20:53:44Z

server/Dockerfile

+WORKDIR /app
+
+# Command to run the application
+CMD ["uvicorn", "server.main:app", "--host", "0.0.0.0", "--port", "8000"]


cmccully · 2026-02-10T20:55:24Z

tests/fastapi/test_admin.py

+        assert response.status_code == 401
+
+
+if __name__ == "__main__":


Does pytest really need this? Doesn't pytest basically automatically run as main by default?

cmccully · 2026-02-10T20:56:18Z

tests/fastapi/test_candidate.py

+        assert candidate["associated_galaxy_redshift"] == 0.1
+
+
+if __name__ == "__main__":


Same that I don't think we need a main.

cmccully · 2026-02-10T20:57:01Z

tests/fastapi/test_doi.py

+        else:
+            # DOI creation failed (expected in test), so pointing won't be in DOI list
+            # This is acceptable - we've verified the endpoints work correctly
+            pass


Does this ever have the potential to mask a failure?

cmccully · 2026-02-10T20:57:20Z

tests/fastapi/test_doi.py

+        assert response.status_code == status.HTTP_401_UNAUTHORIZED
+
+
+if __name__ == "__main__":


Main probably not necessary.

cmccully · 2026-02-10T20:59:09Z

tests/fastapi/test_event.py

+            )
+
+
+if __name__ == "__main__":


Pytest main

cmccully · 2026-02-10T20:59:35Z

tests/fastapi/test_gw_alert.py

+                    assert False, f"Time field {time_field} is not in ISO 8601 format"
+
+
+if __name__ == "__main__":


Pytest main.

cmccully · 2026-02-10T21:00:40Z

tests/fastapi/test_gw_galaxy.py

+        )
+
+
+if __name__ == "__main__":


Pytest main.

cmccully · 2026-02-10T21:00:51Z

tests/fastapi/test_health.py

+            assert "error" in redis
+
+
+if __name__ == "__main__":


Pytest main.

cmccully · 2026-02-10T21:01:11Z

tests/fastapi/test_icecube.py

+        assert response.status_code == status.HTTP_401_UNAUTHORIZED
+
+
+if __name__ == "__main__":


Pytest main.

cmccully · 2026-02-10T21:01:46Z

tests/fastapi/test_instrument.py

+        assert len([inst for inst in named_insts if inst["id"] in created_ids]) >= 3
+
+
+if __name__ == "__main__":


Pytest main.

cmccully · 2026-02-10T21:03:13Z

tests/fastapi/test_query_alert.py

+            assert result[0]["prob_bns"] > 0.9
+
+
+if __name__ == "__main__":


Pytest main.

cmccully · 2026-02-10T21:03:35Z

tests/fastapi/test_ui.py

+            assert response.status_code == 401
+
+
+if __name__ == "__main__":


Pytest main.

cmccully · 2026-02-10T21:03:57Z

tests/requirements.txt

@@ -0,0 +1,11 @@
+# Test requirements for GWTM FastAPI tests


This should go into the pyproject.toml file. requirements.txt is no longer the standard.

cmccully · 2026-02-10T21:04:04Z

tests/requirements.txt

+fastapi>=0.103.0
+
+# HTTP requests for API testing
+requests>=2.31.0


cmccully · 2026-02-10T21:04:15Z

tests/run-fastapi-tests.sh

+  fi
+fi
+
+echo "All tests completed successfully!"


cmccully · 2026-02-10T21:04:45Z

tests/run-fastapi-tests.sh

+#!/bin/bash
+# Run tests for a specific module
+# Test data is loaded automatically by conftest.py
+set -e


Isn't this just a pytest command? I don't really understand why we need an extra bash script here.

cmccully

I think overall this is tremendous work to get all of this migrated over to fast api and to get the tests all setup. I have made a variety of code readability/ maintainability comments.

Two main concerns:
How does this fit into the gwtm_api repo? Does this replace that repo?

I know we wanted to maintain backward compatibility originally, so we kept the Flask stuff in, but what is the plan to remove all the redundant code now that this is finished? That's a huge mental burden to leave all that code in, and I'd like to remove it while we remember what needs to be removed.

Fixes bugs, removes dead code & improves code quality based on reviewer feedback from Curtis, Austin, and Copilot. - Fix InstrumentType to instrument_type attribute references (3 files) - Delete unused test_refactoring.py and remove from router/README - Complete send_password_reset_email SMTP logic in email.py - Add fastapi to root requirements.txt - Improve skymap exception handling with proper logging - Reduce logging verbosity (DEBUG to INFO) - Remove pytest.main() blocks from all 11 test files - Use scientific notation in frequencyunits.py - Remove unused variables and unnecessary assignments - Replace bare except clauses with specific exception types - Add __all__ to models __init__.py - Remove unused imports (secrets, Path, io)

- Split function.py into focused modules (type_checks, geometry, doi, formatters) - Add AlertRole StrEnum for type-safe alert role comparisons - Refactor candidate filters to data-driven pattern with operator lookups - Replace grade calculator if/elif chains with threshold table lookups - Replace repeated probability rounding with getattr/setattr loop - Use strftime for date formatting in delete_test_alerts - Add local filesystem storage backend to gwtm_io - Add @contextmanager db_session() wrapper, delete unused db/utils.py - Migrate test deps to pyproject.toml, update CI workflow

sfoale added 9 commits May 8, 2025 16:17

Adding fastapi server. Work in progress.

fe3c8e7

Add test data and instrument endpoint tests.

92ba6f6

Add pointing tests. Work in progress

4da60f9

Add candidate and gw_alert tests.

08e90ad

Add more tests.

4fd6a0b

More tests completed.

260e930

All tests complete.

76d5abc

Complete test coverage of fastapi endpoints.

93ba921

Refactor routes. Add README.

Merge branch 'master' into fastapi

f94b067

sfoale requested review from Fingel, cmccully and swyatt7 June 9, 2025 15:16

Missing test.

f7045c5

Fingel reviewed Jun 11, 2025

View reviewed changes

requirements.txt

Flask-Mail==0.10.0

Flask-SQLAlchemy==2.5.1

Flask-WTF

flask-caching

Copy link

Contributor

Fingel Jun 11, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Missing fastapi

Update README with better signposting for running and testing fastapi…

4d0c79d

… application.

sfoale requested a review from Fingel June 12, 2025 15:10

Correct paths

dde0893

cmccully requested a review from Copilot June 16, 2025 19:19

Copilot AI reviewed Jun 16, 2025

View reviewed changes

Copilot review comments.

b3e0821

cmccully reviewed Jul 2, 2025

View reviewed changes

sfoale added 7 commits July 3, 2025 06:24

Add a github action to run the fastapi tests.

a2470c2

PEP 8 formatting with black.

3203d15

fix skaffold run in github action

4a29345

Use docker compose for the testing github action.

6fdabfb

Use docker compose.

0ab1cb7

Initialiase db for github action tests.

8142eed

Create db tables from models with sqlalchemy.

71cda89