Skip to content
Open
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
22 changes: 22 additions & 0 deletions src/controllers/flight.py
Original file line number Diff line number Diff line change
Expand Up @@ -145,6 +145,28 @@ async def get_rocketpy_flight_rpy(
flight_service = FlightService.from_flight_model(flight.flight)
return flight_service.get_flight_rpy()

@controller_exception_handler
async def get_flight_kml(
self,
flight_id: str,
) -> bytes:
"""
Get the flight trajectory as a KML file.

Args:
flight_id: str

Returns:
bytes (KML XML)

Raises:
HTTP 404 Not Found: If the flight is not found
in the database.
"""
flight = await self.get_flight_by_id(flight_id)
flight_service = FlightService.from_flight_model(flight.flight)
return flight_service.get_flight_kml()
Comment on lines +166 to +168

@coderabbitai coderabbitai Bot Jul 25, 2026

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

🚀 Performance & Scalability | 🟠 Major | 🏗️ Heavy lift

🧩 Analysis chain

🏁 Script executed:

#!/bin/bash
set -euo pipefail

echo "== locate files =="
git ls-files | rg '(^|/)src/controllers/flight\.py$|src/\w+/service.*rocket|FlightService|flight_service|flight\.py|api\.py' || true

echo "== outline controller =="
ast-grep outline src/controllers/flight.py --view expanded || true

echo "== relevant controller lines =="
sed -n '130,190p' src/controllers/flight.py

echo "== search FlightService and KML calls =="
rg -n "class FlightService|from_flight_model|get_flight_kml|get_flight_kml\(|RocketPy|FlightService" src tests || true

Repository: RocketPy-Team/Infinity-API

Length of output: 13835


🏁 Script executed:

#!/bin/bash
set -euo pipefail

echo "== FlightService relevant implementation =="
sed -n '47,100p' src/services/flight.py
sed -n '500,575p' src/services/flight.py

echo "== routes controller call =="
sed -n '300,335p' src/routes/flight.py

echo "== dependency pins around rocketpy and async worker packages =="
sed -n '1,220p' pyproject.toml
sed -n '1,220p' requirements.txt 2>/dev/null || true
sed -n '1,220p' requirements.dev.txt 2>/dev/null || true fd -a 'pyproject.toml|requirements.*|poetry.lock|Pipfile.lock' . | sed -n '1,80p'

echo "== inspect installed rocketpy Flight APIs if available =="
python3 - <<'PY'
import importlib.util, inspect
spec = importlib.util.find_spec("rocketpy.simulation.flight")
if spec is None:
    print("rocketpy.simulation.flight not importable in sandbox")
else:
    from rocketpy.simulation.flight import Flight as RocketPyFlight
    print("RocketPyFlight exported")
    for name in dir(RocketPyFlight):
        if "kml" in name.lower() or name == "show_kml":
            print(name)
    if hasattr(RocketPyFlight, "show_kml"):
        print(inspect.signature(RocketPyFlight.show_kml))
PY

Repository: RocketPy-Team/Infinity-API

Length of output: 7661


🌐 Web query:

RocketPy Python FlightDataExporter export_kml source Thread safe

💡 Result:

In RocketPy, the FlightDataExporter class is the recommended way to export simulation data, including KML files for trajectory visualization [1][2]. The export_kml method is accessed by instantiating FlightDataExporter with a Flight object and calling the method with the desired parameters, such as file_name and altitude_mode [3][1]. Regarding thread safety, the FlightDataExporter.export_kml method is not explicitly documented as thread-safe. RocketPy's general architecture, particularly its core simulation classes, is not designed with inherent thread safety for concurrent access to the same object instance [4][5]. While the MonteCarlo simulation engine supports parallel execution, it achieves this by distributing independent Flight instances across separate processes using Python's multiprocessing module rather than by sharing state across threads [4][5]. If you need to perform concurrent exports, it is safer to ensure that each thread or process operates on its own dedicated FlightDataExporter instance and corresponding Flight object to avoid race conditions or data corruption [4][5].

Citations:


Offload KML generation from the event loop.

get_flight_kml() holds the FastAPI event loop while constructing a RocketPy Flight, running the synchronous FlightDataExporter.export_kml() path, and reading/writing temp files. Run the export in a bounded worker pool, such as asyncio.to_thread, so concurrent downloads don’t block unrelated requests. If export state is shared per FlightDataExporter, ensure the export operates on the dedicated exporter/flight instance in that worker.

🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@src/controllers/flight.py` around lines 166 - 168, Update the KML generation
flow in the controller method around FlightService.from_flight_model and
get_flight_kml so the synchronous export runs via asyncio.to_thread or an
equivalent bounded worker-pool mechanism, keeping the event loop non-blocking.
Ensure the worker uses the dedicated FlightService/FlightDataExporter instance
for that flight and preserve the returned KML result.

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

not sure, my memory on the event loop is lacking, but IIRC FastAPI already handles everything async anyway , it will just wrap the sync work in a thread inside the pool and let it compete in scheduling with everything else, can you confirm @coderabbitai ?

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

🧩 Analysis chain

🤖 web_search executed:

FastAPI documentation async def synchronous code called inside async path operation function threadpool Starlette run_in_threadpool

Length of output: 20825


@GabrielBarberini, confirmed by the FastAPI docs: it automatically runs a path operation declared with ordinary def in an external thread pool, but it does not automatically offload synchronous utility calls made from an async def handler.

Here, get_flight_kml() is async def, so the direct synchronous call:

return flight_service.get_flight_kml()

runs on the event-loop thread and blocks it until RocketPy finishes exporting and the temporary-file I/O completes. Explicitly awaiting asyncio.to_thread(...) or fastapi.concurrency.run_in_threadpool(...) is the appropriate fix.

The docs also explicitly note that this automatic thread-pool behavior applies to synchronous path operations and dependencies—not arbitrary synchronous functions invoked within an async path operation: https://fastapi.tiangolo.com/async/.

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

@coderabbitai what would you suggest then ? should we declare this route in specific as sync (def) or keep with it async ? what would you do in case we keep it async to solve that problem?

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

@GabrielBarberini, I’d keep the route and controller async. They already await database work (get_flight_by_id), so changing this endpoint to def would require awkwardly bridging back into async code and provides no benefit.

Offload only the blocking RocketPy export call to FastAPI/Starlette’s managed thread pool:

from starlette.concurrency import run_in_threadpool
`@controller_exception_handler`
async def get_flight_kml(
    self,
    flight_id: str,
) -> bytes:
    flight = await self.get_flight_by_id(flight_id)
    flight_service = FlightService.from_flight_model(flight.flight)

    return await run_in_threadpool(flight_service.get_flight_kml)

This keeps database access on the async path while moving the synchronous RocketPy processing and temporary-file I/O off the event-loop thread. The FlightService is local to this request, so the worker operates on a dedicated service/exporter instance.

asyncio.to_thread(flight_service.get_flight_kml) would also work, but I’d prefer run_in_threadpool(...) in this FastAPI application because it uses Starlette/AnyIO’s managed worker-pool behavior.

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

@aasitvora99 , @Gui-FernandesBR , I think run_in_threadpool(flight_service.get_flight_kml) seems to be a clean solution, wdyt?


@controller_exception_handler
async def get_flight_simulation(
self,
Expand Down
36 changes: 36 additions & 0 deletions src/routes/flight.py
Original file line number Diff line number Diff line change
Expand Up @@ -301,6 +301,42 @@ async def update_flight_rocket(
)


@router.get(
"/{flight_id}/kml",
responses={
200: {
"description": "KML trajectory file download",
"content": {"application/vnd.google-earth.kml+xml": {}},
}
},
status_code=200,
response_class=Response,
)
async def get_flight_kml(
flight_id: str,
controller: FlightControllerDep,
):
"""
Export a flight trajectory as a KML file for Google Earth.

## Args
``` flight_id: str ```
"""
with tracer.start_as_current_span("get_flight_kml"):
kml = await controller.get_flight_kml(flight_id)
headers = {
"Content-Disposition": (
f'attachment; filename="flight_{flight_id}.kml"'
),
}
return Response(
content=kml,
headers=headers,
media_type="application/vnd.google-earth.kml+xml",
status_code=200,
)


@router.get("/{flight_id}/simulate")
async def get_flight_simulation(
flight_id: str,
Expand Down
22 changes: 22 additions & 0 deletions src/services/flight.py
Original file line number Diff line number Diff line change
@@ -1,9 +1,12 @@
import json
import os
import tempfile
from typing import Self, Tuple

import numpy as np

from rocketpy.simulation.flight import Flight as RocketPyFlight
from rocketpy.simulation.flight_data_exporter import FlightDataExporter
from rocketpy._encoders import RocketPyEncoder, RocketPyDecoder
from rocketpy.mathutils.function import Function
from rocketpy.motors.solid_motor import SolidMotor
Expand Down Expand Up @@ -499,6 +502,25 @@ def get_flight_simulation(self) -> FlightSimulation:
flight_simulation = FlightSimulation(**encoded_attributes)
return flight_simulation

def get_flight_kml(self) -> bytes:
"""
Get the flight trajectory as a KML file for Google Earth.

Returns:
bytes (UTF-8 encoded KML)
"""
with tempfile.NamedTemporaryFile(
suffix=".kml", delete=False
) as tmp:
tmp_path = tmp.name
try:
FlightDataExporter(self.flight).export_kml(file_name=tmp_path)
with open(tmp_path, "rb") as fh:
return fh.read()
finally:
if os.path.exists(tmp_path):
os.unlink(tmp_path)

def get_flight_rpy(self) -> bytes:
"""
Get the portable JSON ``.rpy`` representation of the flight.
Expand Down
33 changes: 33 additions & 0 deletions tests/unit/test_routes/test_flights_route.py
Original file line number Diff line number Diff line change
Expand Up @@ -57,6 +57,7 @@ def mock_controller_instance():
mock_controller.get_rocketpy_flight_rpy = AsyncMock()
mock_controller.import_flight_from_rpy = AsyncMock()
mock_controller.get_flight_notebook = AsyncMock()
mock_controller.get_flight_kml = AsyncMock()
mock_controller.update_environment_by_flight_id = AsyncMock()
mock_controller.update_rocket_by_flight_id = AsyncMock()
mock_controller.create_flight_from_references = AsyncMock()
Expand Down Expand Up @@ -539,6 +540,38 @@ def test_read_rocketpy_flight_rpy_server_error(mock_controller_instance):
assert response.json() == {'detail': 'Internal Server Error'}


def test_read_flight_kml(mock_controller_instance):
kml_bytes = b'<?xml version="1.0" encoding="UTF-8"?><kml></kml>'
mock_controller_instance.get_flight_kml = AsyncMock(return_value=kml_bytes)
response = client.get('/flights/123/kml')
assert response.status_code == 200
assert response.content == kml_bytes
assert (
response.headers['content-type']
== 'application/vnd.google-earth.kml+xml'
)
assert 'flight_123.kml' in response.headers['content-disposition']
mock_controller_instance.get_flight_kml.assert_called_once_with('123')


def test_read_flight_kml_not_found(mock_controller_instance):
mock_controller_instance.get_flight_kml.side_effect = HTTPException(
status_code=status.HTTP_404_NOT_FOUND
)
response = client.get('/flights/123/kml')
assert response.status_code == 404
assert response.json() == {'detail': 'Not Found'}


def test_read_flight_kml_server_error(mock_controller_instance):
mock_controller_instance.get_flight_kml.side_effect = HTTPException(
status_code=status.HTTP_500_INTERNAL_SERVER_ERROR
)
response = client.get('/flights/123/kml')
assert response.status_code == 500
assert response.json() == {'detail': 'Internal Server Error'}


# --- Issue #56: Import flight from .rpy ---


Expand Down
10 changes: 5 additions & 5 deletions tests/unit/test_routes/test_motors_route.py
Original file line number Diff line number Diff line change
Expand Up @@ -249,7 +249,7 @@ def test_create_liquid_motor_sampled_density(


def test_create_motor_invalid_geometry_kind(
stub_motor_dump, stub_tank_dump, mock_controller_instance
stub_motor_dump, stub_tank_dump
):
stub_tank_dump['geometry'] = {
'geometry_kind': 'pyramid',
Expand All @@ -263,7 +263,7 @@ def test_create_motor_invalid_geometry_kind(


def test_create_motor_mass_kind_missing_fields(
stub_motor_dump, stub_tank_dump, mock_controller_instance
stub_motor_dump, stub_tank_dump
):
# stub_tank_dump defaults to MASS_FLOW with all required fields
# populated; switching to MASS without adding liquid_mass/gas_mass
Expand All @@ -280,7 +280,7 @@ def test_create_motor_mass_kind_missing_fields(


def test_create_motor_level_kind_missing_liquid_height(
stub_motor_dump, stub_tank_dump, mock_controller_instance
stub_motor_dump, stub_tank_dump
):
stub_tank_dump['tank_kind'] = 'LEVEL'
stub_motor_dump.update(
Expand All @@ -292,7 +292,7 @@ def test_create_motor_level_kind_missing_liquid_height(


def test_create_motor_ullage_kind_missing_ullage(
stub_motor_dump, stub_tank_dump, mock_controller_instance
stub_motor_dump, stub_tank_dump
):
stub_tank_dump['tank_kind'] = 'ULLAGE'
stub_motor_dump.update(
Expand All @@ -304,7 +304,7 @@ def test_create_motor_ullage_kind_missing_ullage(


def test_create_motor_mass_flow_kind_missing_flow_rates(
stub_motor_dump, mock_controller_instance
stub_motor_dump
):
# Build a tank payload with MASS_FLOW kind but no flow-rate fields
# so the guard rejects it.
Expand Down
Loading