BlueOS icon indicating copy to clipboard operation
BlueOS copied to clipboard

core: services: Move from pykson to pydantic

Open nicoschmdt opened this issue 4 months ago • 1 comments

fix: #2015

Had to create more tests to attain +75% of code coverage

Summary by Sourcery

Migrate all core service settings and managers from pykson to pydantic, remove legacy pykson support, and introduce new unit tests for firmware uploading to improve coverage.

Enhancements:

  • Migrate service settings schemas from pykson JsonObject to pydantic BaseModel and PydanticSettings
  • Replace PyksonManager with PydanticManager across services and update Manager references
  • Remove pykson dependency from project and related source code

Build:

  • Remove pykson package from project dependencies in pyproject.toml

Tests:

  • Add comprehensive unit tests for FirmwareUploader to validate initialization, binary validation, configuration setters, and upload flow, increasing code coverage above 75%

nicoschmdt avatar Oct 22 '25 18:10 nicoschmdt

Reviewer's Guide

This PR replaces pykson-based settings models and managers across services with pydantic-based implementations, removes all pykson artifacts and dependencies, updates migration and serialization logic accordingly, and adds comprehensive tests for the FirmwareUploader to boost coverage.

Entity relationship diagram for updated settings data structures

erDiagram
    SETTINGSV1 {
        int VERSION
        DefaultSettings default
        string[] blacklist
        Interface[] interfaces
        ServiceTypes[] advertisement_types
    }
    DefaultSettings {
        string[] domain_names
        string[] advertise
        string ip
    }
    Interface {
        string name
        string[] domain_names
        string[] advertise
        string ip
    }
    ServiceTypes {
        string name
        string protocol
        int port
        string properties
    }
    SETTINGSV1 ||--o| DefaultSettings : has
    SETTINGSV1 ||--|{ Interface : has
    SETTINGSV1 ||--|{ ServiceTypes : has
    Interface ||--o| DefaultSettings : has

    EXTENSIONS {
        string identifier
        string name
        string docker
        string tag
        string permissions
        bool enabled
        string user_permissions
    }
    MANIFESTS {
        string identifier
        bool enabled
        int priority
        bool factory
        string name
        string url
    }
    KRAKENSETTINGSV1 {
        int VERSION
        EXTENSIONS[] extensions
    }
    KRAKENSETTINGSV2 {
        int VERSION
        MANIFESTS[] manifests
    }
    KRAKENSETTINGSV2 ||--|{ MANIFESTS : has
    KRAKENSETTINGSV1 ||--|{ EXTENSIONS : has

    BRIDGESETTINGSV1 {
        int VERSION
        BridgeSettingsSpecV1[] specs
    }
    BridgeSettingsSpecV1 {
        string serial_path
        int baudrate
        string ip
        int udp_port
    }
    BRIDGESETTINGSV1 ||--|{ BridgeSettingsSpecV1 : has
    BRIDGESETTINGSV2 {
        int VERSION
        BridgeSettingsSpecV2[] specsv2
    }
    BridgeSettingsSpecV2 {
        int udp_target_port
        int udp_listen_port
        string serial_path
        int baudrate
        string ip
    }
    BRIDGESETTINGSV2 ||--|{ BridgeSettingsSpecV2 : has

    NMEAINJECTORSETTINGSV1 {
        int VERSION
        NmeaInjectorSettingsSpecV1[] specs
    }
    NmeaInjectorSettingsSpecV1 {
        string kind
        int port
        int component_id
    }
    NMEAINJECTORSETTINGSV1 ||--|{ NmeaInjectorSettingsSpecV1 : has

    PINGSETTINGSV1 {
        int VERSION
        Ping1dSettingsSpecV1[] ping1d_specs
    }
    Ping1dSettingsSpecV1 {
        string port
        bool mavlink_enabled
    }
    PINGSETTINGSV1 ||--|{ Ping1dSettingsSpecV1 : has

    WIFISETTINGSV1 {
        int VERSION
        bool hotspot_enabled
        string hotspot_ssid
        string hotspot_password
        bool smart_hotspot_enabled
    }

Class diagram for updated settings models (pykson → pydantic)

classDiagram
    class DefaultSettings {
        +List[str] domain_names
        +List[str] advertise
        +str ip
    }
    class Interface {
        +str name
        +List[str] domain_names
        +List[str] advertise
        +str ip
        +get_phys()
    }
    class ServiceTypes {
        +str name
        +Literal["_udp", "_tcp"] protocol
        +int port
        +Optional[str] properties
        +get_properties()
    }
    class SettingsV1 {
        +int VERSION = 1
        +DefaultSettings default
        +List[str] blacklist
        +List[Interface] interfaces
        +List[ServiceTypes] advertisement_types
        +migrate(data)
        +get_interface_or_create_default(name)
    }
    class SettingsV2 {
        +int VERSION = 2
        +migrate(data)
    }
    class SettingsV3 {
        +int VERSION = 3
        +migrate(data)
    }
    class SettingsV4 {
        +int VERSION = 4
        +str vehicle_name
        +migrate(data)
    }
    SettingsV2 --|> SettingsV1
    SettingsV3 --|> SettingsV2
    SettingsV4 --|> SettingsV3
    SettingsV1 o-- DefaultSettings
    SettingsV1 o-- Interface
    SettingsV1 o-- ServiceTypes

    class ExtensionSettings {
        +str identifier
        +str name
        +str docker
        +str tag
        +str permissions
        +bool enabled
        +str user_permissions
        +settings()
        +container_name()
    }
    class ManifestSettings {
        +str identifier
        +bool enabled
        +int priority
        +bool factory
        +str name
        +str url
    }
    class KrakenSettingsV1 {
        +int VERSION = 1
        +Sequence[ExtensionSettings] extensions
        +migrate(data)
    }
    class KrakenSettingsV2 {
        +int VERSION = 2
        +Sequence[ManifestSettings] manifests
        +migrate(data)
    }
    KrakenSettingsV2 --|> KrakenSettingsV1
    KrakenSettingsV1 o-- ExtensionSettings
    KrakenSettingsV2 o-- ManifestSettings

    class BridgeSettingsSpecV1 {
        +str serial_path
        +int baudrate
        +str ip
        +int udp_port
        +from_spec(spec)
        +__eq__(other)
    }
    class BridgeSettingsSpecV2 {
        +int udp_target_port
        +int udp_listen_port
        +str serial_path
        +int baudrate
        +str ip
        +from_spec(spec)
        +__eq__(other)
    }
    class BridgetSettingsV1 {
        +int VERSION = 1
        +List[BridgeSettingsSpecV1] specs
        +migrate(data)
    }
    class BridgetSettingsV2 {
        +int VERSION = 2
        +List[BridgeSettingsSpecV2] specsv2
        +migrate(data)
    }
    BridgetSettingsV2 --|> BridgetSettingsV1
    BridgetSettingsV1 o-- BridgeSettingsSpecV1
    BridgetSettingsV2 o-- BridgeSettingsSpecV2

    class NmeaInjectorSettingsSpecV1 {
        +str kind
        +int port
        +int component_id
        +__eq__(other)
    }
    class NmeaInjectorSettingsV1 {
        +int VERSION = 1
        +List[NmeaInjectorSettingsSpecV1] specs
        +migrate(data)
    }
    NmeaInjectorSettingsV1 o-- NmeaInjectorSettingsSpecV1

    class Ping1dSettingsSpecV1 {
        +str port
        +bool mavlink_enabled
        +__str__()
        +new(port, enabled)
    }
    class PingSettingsV1 {
        +int VERSION = 1
        +List[Ping1dSettingsSpecV1] ping1d_specs
        +migrate(data)
    }
    PingSettingsV1 o-- Ping1dSettingsSpecV1

    class WifiSettingsV1 {
        +int VERSION = 1
        +bool hotspot_enabled
        +str hotspot_ssid
        +str hotspot_password
        +bool smart_hotspot_enabled
        +migrate(data)
    }

Class diagram for updated settings manager usage

classDiagram
    class PydanticManager {
        +settings
        +load()
        +save()
        +load_from_file()
    }
    class Beacon {
        +PydanticManager manager
        +load_default_settings()
    }
    class Bridget {
        +PydanticManager _settings_manager
    }
    class Kraken {
        +PydanticManager _manager
    }
    class ManifestManager {
        +PydanticManager _manager
    }
    class TrafficController {
        +PydanticManager _settings_manager
    }
    class Ping1DDriver {
        +PydanticManager manager
    }
    class AbstractWifiManager {
        +PydanticManager _settings_manager
    }
    Beacon o-- PydanticManager
    Bridget o-- PydanticManager
    Kraken o-- PydanticManager
    ManifestManager o-- PydanticManager
    TrafficController o-- PydanticManager
    Ping1DDriver o-- PydanticManager
    AbstractWifiManager o-- PydanticManager

File-Level Changes

Change Details Files
Migrate settings and spec models from pykson.JsonObject to pydantic.BaseModel
  • Change class inheritance to BaseModel
  • Annotate fields with Python types (List, Optional, Literal, etc.)
  • Remove pykson field declarations and custom constructors
core/services/beacon/settings.py
core/services/kraken/settings.py
core/services/bridget/settings.py
core/services/nmea_injector/nmea_injector/settings.py
core/services/ping/settings.py
core/services/wifi/settings.py
Switch settings infrastructure to PydanticSettings and PydanticManager
  • Import and extend PydanticSettings instead of BaseSettings
  • Replace Manager(...) instantiations with PydanticManager(...)
  • Update type hints and remove BaseSettings references
core/services/beacon/settings.py
core/services/beacon/main.py
core/services/bridget/bridget.py
core/services/kraken/extension/extension.py
core/services/kraken/kraken.py
core/services/kraken/manifest/manifest.py
core/services/nmea_injector/nmea_injector/TrafficController.py
core/services/ping/ping1d_driver.py
core/services/wifi/wifi_handlers/AbstractWifiHandler.py
core/libs/commonwealth/src/commonwealth/settings/manager.py
Remove pykson imports, classes, and dependency entries
  • Delete pykson_base and pykson_manager modules
  • Remove pykson from pyproject.toml dependencies and uv sources
  • Drop all pykson imports in settings and manager code
core/libs/commonwealth/pyproject.toml
core/libs/commonwealth/src/commonwealth/settings/bases/pykson_base.py
core/libs/commonwealth/src/commonwealth/settings/managers/pykson_manager.py
core/libs/commonwealth/src/commonwealth/settings/tests/test_manager.py
core/libs/commonwealth/src/commonwealth/settings/tests/test_settings.py
Adjust migration and serialization logic for pydantic
  • Use BaseModel.dict() instead of internal _data
  • Handle Optional properties in get_properties implementations
  • Update version fields to class attributes
core/services/beacon/settings.py
Add comprehensive tests for FirmwareUploader
  • Implement pytest unit tests covering init, validation, setter methods
  • Use AsyncMock to simulate subprocess behavior and verify async upload flow
core/services/ardupilot_manager/firmware/test_FirmwareUpload.py

Tips and commands

Interacting with Sourcery

  • Trigger a new review: Comment @sourcery-ai review on the pull request.
  • Continue discussions: Reply directly to Sourcery's review comments.
  • Generate a GitHub issue from a review comment: Ask Sourcery to create an issue from a review comment by replying to it. You can also reply to a review comment with @sourcery-ai issue to create an issue from it.
  • Generate a pull request title: Write @sourcery-ai anywhere in the pull request title to generate a title at any time. You can also comment @sourcery-ai title on the pull request to (re-)generate the title at any time.
  • Generate a pull request summary: Write @sourcery-ai summary anywhere in the pull request body to generate a PR summary at any time exactly where you want it. You can also comment @sourcery-ai summary on the pull request to (re-)generate the summary at any time.
  • Generate reviewer's guide: Comment @sourcery-ai guide on the pull request to (re-)generate the reviewer's guide at any time.
  • Resolve all Sourcery comments: Comment @sourcery-ai resolve on the pull request to resolve all Sourcery comments. Useful if you've already addressed all the comments and don't want to see them anymore.
  • Dismiss all Sourcery reviews: Comment @sourcery-ai dismiss on the pull request to dismiss all existing Sourcery reviews. Especially useful if you want to start fresh with a new review - don't forget to comment @sourcery-ai review to trigger a new review!

Customizing Your Experience

Access your dashboard to:

  • Enable or disable review features such as the Sourcery-generated pull request summary, the reviewer's guide, and others.
  • Change the review language.
  • Add, remove or edit custom review instructions.
  • Adjust other review settings.

Getting Help

  • Contact our support team for questions or feedback.
  • Visit our documentation for detailed guides and information.
  • Keep in touch with the Sourcery team by following us on X/Twitter, LinkedIn or GitHub.

sourcery-ai[bot] avatar Oct 22 '25 18:10 sourcery-ai[bot]