-
Notifications
You must be signed in to change notification settings - Fork 72
Refactor metadata_manager to move to versions provider based model
#244
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
Open
shiv-tyagi
wants to merge
8
commits into
ArduPilot:main
Choose a base branch
from
shiv-tyagi:feat/manifest-provider
base: main
Could not load branches
Branch not found: {{ refName }}
Loading
Could not load tags
Nothing to show
Loading
Are you sure you want to change the base?
Some commits from the old base branch may be removed from the timeline,
and old review comments may become outdated.
Open
Changes from all commits
Commits
Show all changes
8 commits
Select commit
Hold shift + click to select a range
db4eb0c
metadata_manager: refactor to a provider based versions manager
shiv-tyagi cd2e41d
scripts: drop cron based version updation scripts
shiv-tyagi 990f8a6
web: use new provider based versions manager
shiv-tyagi 02d606c
docker-compose: rename CBS_REMOTES_RELOAD_TOKEN to CBS_ADMIN_TOKEN
shiv-tyagi 83f5a28
tests: add/update tests for provider based versions manager
shiv-tyagi b39cad9
builder: add requests and packaging as requirements
shiv-tyagi 6848505
examples: use vehicle id in remotes.json instead of vehicle name
shiv-tyagi 23eb457
README: update docs to reflect remotes.json is now optional
shiv-tyagi File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -1,3 +1,5 @@ | ||
| jsonschema==4.20.0 | ||
| redis==5.2.1 | ||
| dill==0.3.8 | ||
| requests==2.31.0 | ||
| packaging==26.2 | ||
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -1,21 +1,39 @@ | ||
| from .versions_fetcher import ( | ||
| VersionsFetcher, | ||
| from .ap_src_meta_fetcher import APSourceMetadataFetcher | ||
| from .firmware_server import ManifestJson, ManifestFetchError, ReleaseRecord | ||
| from .firmware_server.client import ManifestClient | ||
| from .firmware_server.index import ManifestIndex | ||
| from .vehicles_manager import DEFAULT_VEHICLES, Vehicle, VehiclesManager | ||
| from .versions_manager import ( | ||
| DEFAULT_WHITELISTED_FORK_REMOTES, | ||
| ForkRemoteSpec, | ||
| ManifestJsonVersionsProvider, | ||
| RemoteInfo, | ||
| ) | ||
|
|
||
| from .ap_src_meta_fetcher import ( | ||
| APSourceMetadataFetcher, | ||
| ) | ||
|
|
||
| from .vehicles_manager import ( | ||
| VehiclesManager, | ||
| Vehicle, | ||
| RemotesJsonVersionsProvider, | ||
| VersionInfo, | ||
| VersionsManager, | ||
| VersionsProvider, | ||
| WhitelistedForkTagVersionsProvider, | ||
| build_default_providers, | ||
| ) | ||
|
|
||
| __all__ = [ | ||
| "APSourceMetadataFetcher", | ||
| "VersionsFetcher", | ||
| "DEFAULT_VEHICLES", | ||
| "DEFAULT_WHITELISTED_FORK_REMOTES", | ||
| "ForkRemoteSpec", | ||
| "ManifestClient", | ||
| "ManifestFetchError", | ||
| "ManifestIndex", | ||
| "ManifestJson", | ||
| "ManifestJsonVersionsProvider", | ||
| "ReleaseRecord", | ||
| "RemoteInfo", | ||
| "VehiclesManager", | ||
| "RemotesJsonVersionsProvider", | ||
| "Vehicle", | ||
| "VersionInfo", | ||
| "VersionsManager", | ||
| "VersionsProvider", | ||
| "VehiclesManager", | ||
| "WhitelistedForkTagVersionsProvider", | ||
| "build_default_providers", | ||
| ] |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change | ||||
|---|---|---|---|---|---|---|
| @@ -0,0 +1,9 @@ | ||||||
| from .manifest import ManifestJson | ||||||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Should probably be
Suggested change
|
||||||
| from .models import ReleaseRecord | ||||||
| from .exceptions import ManifestFetchError | ||||||
|
|
||||||
| __all__ = [ | ||||||
| "ManifestJson", | ||||||
| "ManifestFetchError", | ||||||
| "ReleaseRecord", | ||||||
| ] | ||||||
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,137 @@ | ||
| import json | ||
| import logging | ||
| from dataclasses import dataclass | ||
| from datetime import datetime, timezone | ||
| from pathlib import Path | ||
| from typing import Optional | ||
|
|
||
| import requests | ||
|
|
||
| from .exceptions import ManifestFetchError | ||
|
|
||
|
|
||
| @dataclass | ||
| class _CacheMeta: | ||
| etag: Optional[str] = None | ||
| last_modified: Optional[str] = None | ||
| fetched_at: Optional[str] = None | ||
|
|
||
| def to_dict(self) -> dict: | ||
| return { | ||
| "etag": self.etag, | ||
| "last_modified": self.last_modified, | ||
| "fetched_at": self.fetched_at, | ||
| } | ||
|
|
||
| @classmethod | ||
| def from_dict(cls, data: dict) -> "_CacheMeta": | ||
| return cls( | ||
| etag=data.get("etag"), | ||
| last_modified=data.get("last_modified"), | ||
| fetched_at=data.get("fetched_at"), | ||
| ) | ||
|
|
||
|
|
||
| class ManifestClient: | ||
| """Fetch and cache the ArduPilot firmware manifest.json file.""" | ||
|
|
||
| def __init__( | ||
| self, | ||
| url: str, | ||
| cache_dir: str, | ||
| timeout: int = 120, | ||
| user_agent: str = "CustomBuild/1.0", | ||
| ): | ||
| self.url = url | ||
| self.cache_dir = Path(cache_dir) | ||
| self.cache_path = self.cache_dir / "manifest.json" | ||
| self.meta_path = self.cache_dir / "manifest.json.meta" | ||
| self.timeout = timeout | ||
| self.user_agent = user_agent | ||
| self.logger = logging.getLogger(__name__) | ||
|
|
||
| def fetch_raw(self) -> bytes: | ||
| headers = {"User-Agent": self.user_agent} | ||
| meta = self._read_meta() if self._has_cache() else _CacheMeta() | ||
|
|
||
| if meta.etag: | ||
| headers["If-None-Match"] = meta.etag | ||
| if meta.last_modified: | ||
| headers["If-Modified-Since"] = meta.last_modified | ||
|
|
||
| try: | ||
| response = requests.get( | ||
| self.url, | ||
| headers=headers, | ||
| timeout=self.timeout, | ||
| ) | ||
| except requests.RequestException as exc: | ||
| return self._fallback_or_raise(exc) | ||
|
|
||
| if response.status_code == 304: | ||
| self.logger.info("Manifest not modified (304), using cache") | ||
| self._touch_fetched_at(self._now_iso()) | ||
| return self._read_cache_bytes() | ||
|
|
||
| if response.status_code != 200: | ||
| return self._fallback_or_raise( | ||
| ManifestFetchError( | ||
| f"Manifest fetch failed with status {response.status_code}" | ||
| ) | ||
| ) | ||
|
|
||
| raw = response.content | ||
| self._write_cache( | ||
| raw, | ||
| _CacheMeta( | ||
| etag=response.headers.get("ETag"), | ||
| last_modified=response.headers.get("Last-Modified"), | ||
| fetched_at=self._now_iso(), | ||
| ), | ||
| ) | ||
| self.logger.info("Downloaded manifest (%d bytes)", len(raw)) | ||
| return raw | ||
|
|
||
| def fetch(self) -> dict: | ||
| return json.loads(self.fetch_raw().decode("utf-8")) | ||
|
|
||
| def _has_cache(self) -> bool: | ||
| return self.cache_path.is_file() | ||
|
|
||
| def _read_cache_bytes(self) -> bytes: | ||
| return self.cache_path.read_bytes() | ||
|
|
||
| def _read_meta(self) -> _CacheMeta: | ||
| if not self.meta_path.is_file(): | ||
| return _CacheMeta() | ||
| return _CacheMeta.from_dict( | ||
| json.loads(self.meta_path.read_text(encoding="utf-8")) | ||
| ) | ||
|
|
||
| def _write_cache(self, raw: bytes, meta: _CacheMeta) -> None: | ||
| self.cache_dir.mkdir(parents=True, exist_ok=True) | ||
| self.cache_path.write_bytes(raw) | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Consider writing into a separate file and |
||
| self.meta_path.write_text( | ||
| json.dumps(meta.to_dict(), indent=2), | ||
| encoding="utf-8", | ||
| ) | ||
|
|
||
| def _touch_fetched_at(self, fetched_at: str) -> None: | ||
| meta = self._read_meta() | ||
| meta.fetched_at = fetched_at | ||
| self.meta_path.write_text( | ||
| json.dumps(meta.to_dict(), indent=2), | ||
| encoding="utf-8", | ||
| ) | ||
|
|
||
| def _fallback_or_raise(self, exc: Exception) -> bytes: | ||
| if self._has_cache(): | ||
| self.logger.warning( | ||
| "Manifest fetch failed (%s), using stale cache", exc | ||
| ) | ||
| return self._read_cache_bytes() | ||
| raise ManifestFetchError(str(exc)) from exc | ||
|
|
||
| @staticmethod | ||
| def _now_iso() -> str: | ||
| return datetime.now(timezone.utc).isoformat() | ||
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,2 @@ | ||
| class ManifestFetchError(Exception): | ||
| """Raised when the firmware manifest cannot be fetched and no cache exists.""" |
Oops, something went wrong.
Oops, something went wrong.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This is a mismatch with the web requirements.txt