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
1 change: 1 addition & 0 deletions builder/requirements.txt
Original file line number Diff line number Diff line change
@@ -1,3 +1,4 @@
jsonschema
redis
dill==0.3.8
beautifulsoup4==4.12.3
62 changes: 57 additions & 5 deletions metadata_manager/ap_src_meta_fetcher.py
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,18 @@
import ap_git
import os

from utils.apache_dir_listing import parse_apache_dir_listing


class FirmwareServerUnavailableError(Exception):
"""Raised when the firmware server cannot be reached or returns an error."""


def _board_artifacts_subdir(board_id: str, vehicle_id: str = None) -> str:
if vehicle_id == "heli":
return f"{board_id}-heli"
return board_id


class APSourceMetadataFetcher:
"""
Expand Down Expand Up @@ -503,11 +515,7 @@ def get_board_defaults_from_fw_server(
"""
import requests

# Heli builds are stored under a separate folder
artifacts_subdir = board_id
if vehicle_id == "Heli":
artifacts_subdir += "-heli"

artifacts_subdir = _board_artifacts_subdir(board_id, vehicle_id)
features_txt_url = f"{artifacts_url}/{artifacts_subdir}/features.txt"

try:
Expand Down Expand Up @@ -551,6 +559,50 @@ def get_board_defaults_from_fw_server(
)
return None

def get_board_standard_artifacts_from_fw_server(

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.

This doesn't look right. Why aren't you using the manifest file? It's there explicitly so this sort of thing is not required.

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

There are a couple of reasons I avoided that this time.

We already have the path for firmware server builds (firmware.ardupilot.org//) for each version listed on the custom build server. We construct it in the version fetcher module.

To get the list of artifacts for a board, we just need to append the board name to it to get the exact path where that board's artifacts are stored.

We have been using this approach for quite some time. It is already used to retrieve the features.txt file. All we need to do to get the list of all versions is extend the same logic to also retrieve the list of files from there.

Using manifests.json is a much larger piece of work. We would need to download a file (over 60 MB today) asynchronously, parse it, group the entries by version and board, and then map them to the versions listed on custom.ardupilot.org (which come directly from GitHub tags).

I agree that this approach would be more correct overall, but in my opinion, extending what we already use for features.txt is the right approach for now.

self,
version_artifacts_url: str,
board_id: str,
vehicle_id: str = None,
) -> list | None:
"""
Fetch standard build artifact file listings from firmware.ardupilot.org.

Parameters:
version_artifacts_url (str): Base URL for build artifacts for a version.
board_id (str): Board identifier
vehicle_id (str): Vehicle identifier (for special handling like Heli)

Returns:
list: File entries with name, url, size, and modified fields.
None: If the board directory does not exist on the firmware server.

Raises:
FirmwareServerUnavailableError: If the firmware server is unreachable
or returns a non-404 error.
"""
import requests

board_artifacts_subdir = _board_artifacts_subdir(board_id, vehicle_id)
listing_url = f"{version_artifacts_url.rstrip('/')}/{board_artifacts_subdir}/"

try:
response = requests.get(listing_url, timeout=30)
if response.status_code == 404:
return None
response.raise_for_status()
return parse_apache_dir_listing(response.text, base_url=listing_url)
except requests.HTTPError as e:
self.logger.warning(
f"Failed to fetch standard artifacts from {listing_url}: {e}"
)
raise FirmwareServerUnavailableError(str(e)) from e
except requests.RequestException as e:
self.logger.warning(
f"Failed to fetch standard artifacts from {listing_url}: {e}"
)
raise FirmwareServerUnavailableError(str(e)) from e

@staticmethod
def get_singleton():
return APSourceMetadataFetcher.__singleton
27 changes: 27 additions & 0 deletions tests/utils/fixtures/apache_dir_listing_sample.html
Original file line number Diff line number Diff line change
@@ -0,0 +1,27 @@
<html>
<body>
<table width=80%>
<tr bgcolor="#aaaaaa"><td width=50 align=center><b>Type</b></td><td><b>Filename</b></td><td><b>Date</b></td><td><b>Size</b></td></tr>
<tr bgcolor="#ffffff"><td align=center><img src="/icons/back.gif"></td><td><a href="/Copter/stable-4.5.0"><b>Parent Directory</B> </a></td>
<td>--</td><td>--</td>
<tr bgcolor="#cacaca">
<td align=center><img src="/icons/text.gif"></td>
<td><a href="/Copter/stable-4.5.0/CubeOrange/arducopter.abin">arducopter.abin</a></td>
<td>Tue Apr 2 05:11:12 2024</td>
<td>1814971</td>
</tr>
<tr bgcolor="#ffffff">
<td align=center><img src="/icons/text.gif"></td>
<td><a href="/Copter/stable-4.5.0/CubeOrange/arducopter.apj">arducopter.apj</a></td>
<td>Tue Apr 15 05:11:12 2024</td>
<td>1640045</td>
</tr>
<tr bgcolor="#cacaca">
<td align=center><img src="/icons/text.gif"></td>
<td><a href="/Copter/stable-4.5.0/CubeOrange/features.txt">features.txt</a></td>
<td>Tue Apr 2 05:11:12 2024</td>
<td>8294</td>
</tr>
</table>
</body>
</html>
60 changes: 60 additions & 0 deletions tests/utils/test_apache_dir_listing.py
Original file line number Diff line number Diff line change
@@ -0,0 +1,60 @@
"""
Tests for Apache directory listing parser.
"""
from datetime import datetime, timezone
from pathlib import Path

from utils.apache_dir_listing import parse_apache_dir_listing, _parse_apache_date


FIXTURES_DIR = Path(__file__).parent / "fixtures"
SAMPLE_LISTING_HTML = (FIXTURES_DIR / "apache_dir_listing_sample.html").read_text()
BASE_URL = "https://firmware.ardupilot.org/Copter/stable-4.5.0/CubeOrange/"


class TestParseApacheDate:
def test_parses_single_digit_day(self):
result = _parse_apache_date("Tue Apr 2 05:11:12 2024")
assert result == datetime(2024, 4, 2, 5, 11, 12, tzinfo=timezone.utc)

def test_parses_double_digit_day(self):
result = _parse_apache_date("Tue Apr 15 05:11:12 2024")
assert result == datetime(2024, 4, 15, 5, 11, 12, tzinfo=timezone.utc)

def test_returns_none_for_dash(self):
assert _parse_apache_date("--") is None

def test_returns_none_for_invalid(self):
assert _parse_apache_date("not a date") is None


class TestParseApacheDirListing:
def test_parses_files_and_skips_parent_directory(self):
entries = parse_apache_dir_listing(SAMPLE_LISTING_HTML, BASE_URL)

assert len(entries) == 3
assert entries[0]["name"] == "arducopter.abin"
assert entries[1]["name"] == "arducopter.apj"
assert entries[2]["name"] == "features.txt"

def test_resolves_absolute_urls(self):
entries = parse_apache_dir_listing(SAMPLE_LISTING_HTML, BASE_URL)

assert entries[0]["url"] == (
"https://firmware.ardupilot.org/Copter/stable-4.5.0/"
"CubeOrange/arducopter.abin"
)

def test_parses_size_and_modified(self):
entries = parse_apache_dir_listing(SAMPLE_LISTING_HTML, BASE_URL)

assert entries[0]["size"] == 1814971
assert entries[0]["modified"] == datetime(
2024, 4, 2, 5, 11, 12, tzinfo=timezone.utc
)
assert entries[1]["modified"] == datetime(
2024, 4, 15, 5, 11, 12, tzinfo=timezone.utc
)

def test_returns_empty_list_when_no_table(self):
assert parse_apache_dir_listing("<html><body></body></html>", BASE_URL) == []
82 changes: 82 additions & 0 deletions tests/web/test_vehicles_api.py
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,7 @@
VehicleBase,
VersionOut,
BoardOut,
StandardArtifactOut,
FeatureOut,
CategoryBase,
FeatureDefault,
Expand Down Expand Up @@ -78,6 +79,18 @@ def dummy_feature(
dependencies=[],
)

@staticmethod
def dummy_standard_artifact(
name="arducopter.apj",
url="https://firmware.ardupilot.org/Copter/stable-4.5.0/CubeOrange/arducopter.apj",
):
return StandardArtifactOut(
name=name,
url=url,
size=1640045,
modified=None,
)

# GET /vehicles

def test_list_vehicles_returns_200_with_vehicle_list(self, client):
Expand Down Expand Up @@ -442,6 +455,75 @@ def test_get_board_method_not_allowed(self, client):
response = method("/api/v1/vehicles/copter/versions/v1/boards/b1")
assert response.status_code == status.HTTP_405_METHOD_NOT_ALLOWED

# GET /vehicles/{vehicle_id}/versions/{version_id}/boards/{board_id}/standard_artifacts

_STANDARD_ARTIFACTS_URL = (
"/api/v1/vehicles/copter/versions/copter-4.5.0-stable/"
"boards/MatekH743/standard_artifacts"
)

def test_list_board_standard_artifacts_returns_200(self, client):
mock_vehicles_service = Mock()
mock_vehicles_service.get_board_standard_artifacts.return_value = [
self.dummy_standard_artifact()
]
with self.override_vehicles_service(client, mock_vehicles_service):
response = client.get(self._STANDARD_ARTIFACTS_URL)

assert response.status_code == status.HTTP_200_OK
assert "application/json" in response.headers["content-type"]

def test_list_board_standard_artifacts_returns_404_when_not_found(self, client):
mock_vehicles_service = Mock()
mock_vehicles_service.get_board_standard_artifacts.return_value = None
with self.override_vehicles_service(client, mock_vehicles_service):
response = client.get(self._STANDARD_ARTIFACTS_URL)

assert response.status_code == status.HTTP_404_NOT_FOUND
assert "MatekH743" in response.json()["detail"]

def test_list_board_standard_artifacts_returns_502_on_fw_server_error(self, client):
from metadata_manager.ap_src_meta_fetcher import FirmwareServerUnavailableError

mock_vehicles_service = Mock()
mock_vehicles_service.get_board_standard_artifacts.side_effect = (
FirmwareServerUnavailableError("connection failed")
)
with self.override_vehicles_service(client, mock_vehicles_service):
response = client.get(self._STANDARD_ARTIFACTS_URL)

assert response.status_code == status.HTTP_502_BAD_GATEWAY

def test_list_board_standard_artifacts_response_schema(self, client):
mock_vehicles_service = Mock()
mock_vehicles_service.get_board_standard_artifacts.return_value = [
self.dummy_standard_artifact()
]
with self.override_vehicles_service(client, mock_vehicles_service):
response = client.get(self._STANDARD_ARTIFACTS_URL)

data = response.json()
assert len(data) == 1
for field in ["name", "url", "size", "modified"]:
assert field in data[0]

def test_list_board_standard_artifacts_service_called_with_correct_ids(self, client):
mock_vehicles_service = Mock()
mock_vehicles_service.get_board_standard_artifacts.return_value = [
self.dummy_standard_artifact()
]
with self.override_vehicles_service(client, mock_vehicles_service):
client.get(self._STANDARD_ARTIFACTS_URL)

mock_vehicles_service.get_board_standard_artifacts.assert_called_once_with(
"copter", "copter-4.5.0-stable", "MatekH743"
)

def test_list_board_standard_artifacts_method_not_allowed(self, client):
for method in [client.post, client.put, client.patch, client.delete]:
response = method(self._STANDARD_ARTIFACTS_URL)
assert response.status_code == status.HTTP_405_METHOD_NOT_ALLOWED

# GET /vehicles/{vehicle_id}/versions/{version_id}/boards/{board_id}/features

_FEATURES_URL = "/api/v1/vehicles/copter/versions/copter-4.5.0-stable/boards/MatekH743/features"
Expand Down
Loading
Loading