Implement initial Assets API client with CLI support #21

eduardiazf · 2025-08-15T08:48:23Z

Summary

This PR implements a complete Assets API client for the Frequenz platform, including a command-line interface for interacting with microgrid assets. The implementation provides a robust foundation for asset management operations with comprehensive error handling and testing.

Changes Made

🚀 New Features

Assets API Client (_client.py): Complete gRPC client implementation for the Assets API
CLI Interface (cli/__main__.py): Command-line tool with microgrid command for retrieving microgrid details
Type System (types.py): Data classes for Microgrid, DeliveryArea, and Location with protobuf integration
Exception Handling (exceptions.py): Custom exception hierarchy with JSON serialization support

🔧 Dependencies & Configuration

Added new core dependencies: frequenz-api-assets, frequenz-api-common, frequenz-client-base, grpcio
Introduced optional cli dependency group with asyncclick
Updated pyproject.toml to include CLI dependencies

🧪 Testing Infrastructure

Comprehensive test suite with pytest fixtures and helpers
Mock factories for creating test objects
Assertion helpers for common validation patterns
Test coverage for various microgrid field combinations and error scenarios

Technical Details

Client Architecture

Extends BaseApiClient for consistent authentication and connection management

CLI Features

Async command execution with proper asyncio event loop management
Environment variable support for API credentials (ASSETS_API_AUTH_KEY, ASSETS_API_SIGN_SECRET)
JSON output formatting for easy integration with other tools

Usage Examples

# Get microgrid details
assests-cli microgrid 123

# With custom server and credentials
asssets-cli --server-url grpc://api.example.com:443 \
  --auth-key your-key --sign-secret your-secret microgrid 123

Testing

Comprehensive test coverage for success and error scenarios
Mock-based testing with proper cleanup and isolation
Parametrized tests for different field combinations

Breaking Changes

Updated dependency structure with new optional groups

Impact

This implementation provides a solid foundation for asset management operations and establishes patterns that can be extended for other asset types in the future. The CLI interface makes it easy for users to interact with the Assets API directly from the command line, while the comprehensive client library enables programmatic access for integration into larger applications.

llucax

The PR looks good to me in general, although I made quite a few comments, mostly to point to some common tools you might not know about or point out some coding style issues, so everything quite minor. Nevertheless it would be nice to address them to keep the coding style consistent and avoid code duplication.

In general I would also recommend to split into more commits for next time, to make reviewing easier. For example you could separate updating dependencies, adding the client class, adding the CLI interface, etc.

It is usually much less demanding, at least when reviewing commit-by-commit, to focus on only one set of small changes than having many unrelated changes in mind while going through diffs and files.

pyproject.toml

src/frequenz/client/assets/_client.py

tests/conftest.py

tests/helpers/assertions.py

tests/helpers/factories.py

llucax · 2025-08-18T10:19:36Z

RELEASE_NOTES.md

-<!-- Here goes notes on how to upgrade from previous versions, including deprecations and what they should be replaced with -->
+**Breaking Changes:**
+
+- Removed deprecated `delete_me` function


This function was never intended to be used, it is just part of the template we use to create repositories, so I wouldn't list it in the release notes, nor in the PR description.

This one is still pending :)

llucax · 2025-08-19T12:05:00Z

It looks like commit 6902612 (#21) is missing a title.

llucax

LGTM in general, just a few very minor comments.

llucax · 2025-08-19T12:12:01Z

src/frequenz/client/assets/types.py

+
+from dataclasses import dataclass
+from datetime import datetime, timezone
+from typing import Any, Optional


Nitpick: There is still one Optional here :)

llucax · 2025-08-19T12:14:02Z

src/frequenz/client/assets/types.py

+    def to_dict(self) -> dict[str, Any]:
+        """
+        Convert the DeliveryArea instance to a dictionary.
+
+        Returns:
+            A dictionary containing the delivery area details.
+        """
+        return {"code": self.code, "code_type": self.code_type}


These to_dict() are not necessary, you can use dataclasses.asdict() on any dataclass.

llucax · 2025-08-19T12:16:35Z

tests/conftest.py

+    # Cleanup: reset ALL mocks automatically
+    mock_stub.reset_mock()
+    mock_microgrid.reset_mock()
+    mock_delivery_area.reset_mock()
+    mock_location.reset_mock()
+    mock_response.reset_mock()


Nitpick: You are creating all the mocks here, why do you need to reset them if they will probably be removed right after you reset them?

llucax · 2025-08-19T12:17:44Z

RELEASE_NOTES.md

-<!-- Here goes notes on how to upgrade from previous versions, including deprecations and what they should be replaced with -->
+**Breaking Changes:**
+
+- Removed deprecated `delete_me` function


This one is still pending :)

llucax · 2025-08-19T12:39:01Z

Not sure why GitHub is not allowing me to resolve conversations, maybe a permissions issue? I will check the permissions on this repo.

llucax · 2025-08-19T12:40:06Z

Added the API and SDK team with write access to match other client repo permissions.

eduardiazf · 2025-08-19T13:29:35Z

It looks like commit 6902612 (#21) is missing a title.

Done, I updated the commit message.

cwasicki · 2025-08-19T13:42:07Z

src/frequenz/client/assets/types.py

+
+    latitude: float
+    longitude: float
+    country_code: str


The country code should be optional

cwasicki · 2025-08-19T13:44:35Z

src/frequenz/client/assets/types.py

+            create_time=pb.create_timestamp.ToDatetime().replace(tzinfo=timezone.utc),
+        )
+
+    def to_json(self) -> str:


What is this for? Does it need to be part of the type?

we can move to cli because it is just using in the function print_microgrid_details

cwasicki · 2025-08-19T13:45:16Z

src/frequenz/client/assets/types.py

+    def to_json(self) -> str:
+        """Convert the Microgrid instance to a JSON string."""
+        microgrid_dict = asdict(self)
+        microgrid_dict["id"] = int(self.id)


Is this future proof? I think @llucax just recently introduced a new type for this.

This is for prevent the following TypeError

You need to implement a custom json serializer in this case: json.dump(asdict(the_dataclass), default=my_serializer). I agree it would be better to put this stuff in the CLI module, as it is only used for the CLI, but didn't want to add more comments to the commits because as @cwasicki said in the future this will be moved to common in the future so we'll have to address it anyway.

cwasicki · 2025-08-19T13:48:03Z

src/frequenz/client/assets/__main__.py

@@ -0,0 +1,186 @@
+# License: MIT


In other clients we put this under a cli subfolder, I would suggest to do this here too. See e.g. https://github.com/frequenz-floss/frequenz-client-reporting-python/tree/v0.x.x/src/frequenz/client/reporting/cli

cwasicki · 2025-08-19T13:50:28Z

src/frequenz/client/assets/__main__.py

+    export ASSETS_API_URL="grpc://api.example.com:5555"
+    export ASSETS_API_AUTH_KEY="your-key"
+    export ASSETS_API_SIGN_SECRET="your-secret"
+    python -m frequenz.client.assets microgrid 123


Have a look at the reporting repo for defining a dedicated script that is installed with the package: https://github.com/frequenz-floss/frequenz-client-reporting-python/blob/0309f57573ec11900665243bda3278fcd36b70d5/pyproject.toml#L40

- Added new dependencies: frequenz-api-assets, frequenz-api-common, frequenz-client-base, and grpcio. - Introduced optional dependency group 'cli' with asyncclick. - Updated existing dependencies for frequenz-client-assets to include 'cli'. - Removed deprecated 'delete_me' function from assets client and updated module docstring. Updates dependencies and cleans up assets client Updates project dependencies to integrate with new APIs and client libraries. Introduces a 'cli' optional dependency group, including `asyncclick`. Removes deprecated function from assets client for a cleaner API. Signed-off-by: eduardiazf <eduardiazf@gmail.com>

- Document new features, breaking changes, and dependencies - Add upgrade instructions and CLI usage details Signed-off-by: eduardiazf <eduardiazf@gmail.com>

Signed-off-by: eduardiazf <eduardiazf@gmail.com>

- Added `frequenz-client-common` as a new dependency in `pyproject.toml`. - Refactored error handling in the Assets API client to use `ApiClientError` instead of `AssetsApiError`. - Updated type hints for `auth_key` and `sign_secret` to support `str | None`. - Improved exception handling in `get_microgrid_details` method. - Updated `Microgrid` class to use `MicrogridId` and `EnterpriseId` types for better type safety. These changes improve the robustness and clarity of the Assets API client while ensuring compatibility with the latest dependencies. Signed-off-by: eduardiazf <eduardiazf@gmail.com>

- Updated `typing-extensions` version in `pyproject.toml` to `>= 4.13.0, < 5`. - Refactored assertions in test files to remove the `AssertionHelpers` class, replacing static methods with direct function calls for clarity and simplicity. - Enhanced the `create_*_mock` functions in factories to include specific names and specs for better debugging. These changes improve dependency management and streamline test assertions for better readability. Signed-off-by: eduardiazf <eduardiazf@gmail.com>

- Added `from __future__ import annotations` to improve type hinting. - Changed the return type of the `stub` property to `PlatformAssetsAsyncStub` for better async support. - Introduced new exceptions `DataLoss` and `EntityAlreadyExists` in the exceptions module. - Updated the `Microgrid` class to ensure integer conversion for `id` and `enterprise_id`. These changes enhance type safety and improve the overall structure of the Assets API client. Signed-off-by: eduardiazf <eduardiazf@gmail.com>

- Added a type ignore comment in the `stub` property to handle the casting from a sync stub to an async stub. - This change ensures compatibility with the async implementation while maintaining type safety. These modifications improve the functionality of the Assets API client by addressing type issues rel Signed-off-by: eduardiazf <eduardiazf@gmail.com>

…ation - Corrected the reference to the `GrpcError` class in the docstring of the `get_microgrid_details` method to point to the appropriate module. - This change improves the accuracy of the documentation and helps users understand the error handling better. Signed-off-by: eduardiazf <eduardiazf@gmail.com>

…d functions - Replaced the `to_dict` methods in `DeliveryArea` and `Location` classes with a new `to_json` method in the `Microgrid` class for improved JSON serialization. - Removed the deprecated `delete_me` function from the codebase. - Updated the `RELEASE_NOTES.md` to reflect these changes and added new required dependencies. These modifications enhance the API's usability and maintainability while ensuring a cleaner codebase. Signed-off-by: eduardiazf <eduardiazf@gmail.com>

- Changed type hints for `delivery_area` and `location` attributes in the `Microgrid` class to support `DeliveryArea | None` and `Location | None`. - This modification enhances type safety and improves code readability. Signed-off-by: eduardiazf <eduardiazf@gmail.com>

- Changed the type hint for `country_code` in the `Location` class from `str` to `str | None` to allow for optional values. - This modification enhances type safety and improves code clarity. Signed-off-by: eduardiazf <eduardiazf@gmail.com>

…precated module - Introduced a new CLI entry point in `pyproject.toml` for the Assets API client, allowing users to interact with the API via command line. - Removed the deprecated `__main__.py` file from the `assets` module, streamlining the codebase and enhancing maintainability. These changes improve the usability of the Assets API client by providing a more accessible command-line interface. Signed-off-by: eduardiazf <eduardiazf@gmail.com>

- Removed the deprecated `to_json` method from the `Microgrid` class. - Updated the `print_microgrid_details` function to directly serialize the `Microgrid` instance using `asdict`, enhancing clarity and maintainability. Signed-off-by: eduardiazf <eduardiazf@gmail.com>

cwasicki

LGTM, thanks a lot!

llucax · 2025-08-20T10:12:18Z

LGTM too, the custom json serializer is only a nice-to-have for me.

- Added @frequenz-floss/python-sdk-team to the CODEOWNERS file alongside the existing api-assets-team, ensuring shared ownership and collaboration on repository changes. Signed-off-by: eduardiazf <eduardiazf@gmail.com>

llucax · 2025-08-20T11:24:00Z

Feel free to force-merge as the codeowners file change won't be effective until it is merged.

eduardiazf requested a review from a team as a code owner August 15, 2025 08:48

github-actions bot added part:tests Affects the unit, integration and performance (benchmarks) tests part:tooling Affects the development tooling (CI, deployment, dependency management, etc.) labels Aug 15, 2025

eduardiazf added cmd:skip-release-notes It is not necessary to update release notes for this PR and removed cmd:skip-release-notes It is not necessary to update release notes for this PR labels Aug 15, 2025

github-actions bot added the part:docs Affects the documentation label Aug 15, 2025

eduardiazf force-pushed the feat/initial-client branch from dbf655f to 0ba57bf Compare August 15, 2025 09:22

eduardiazf requested a review from cwasicki August 15, 2025 10:20

llucax reviewed Aug 18, 2025

View reviewed changes

eduardiazf force-pushed the feat/initial-client branch 3 times, most recently from 1af6457 to b793fb8 Compare August 19, 2025 09:47

eduardiazf requested a review from llucax August 19, 2025 10:32

llucax reviewed Aug 19, 2025

View reviewed changes

eduardiazf force-pushed the feat/initial-client branch 3 times, most recently from 0e11021 to c0e5a24 Compare August 19, 2025 13:28

eduardiazf requested a review from llucax August 19, 2025 13:34

cwasicki reviewed Aug 19, 2025

View reviewed changes

eduardiazf added 7 commits August 19, 2025 19:36

docs: update RELEASE_NOTES.md for Assets API client

4334739

- Document new features, breaking changes, and dependencies - Add upgrade instructions and CLI usage details Signed-off-by: eduardiazf <eduardiazf@gmail.com>

chore(deps): Change max version to 1 for frequenz-api-common

f661955

Signed-off-by: eduardiazf <eduardiazf@gmail.com>

eduardiazf added 6 commits August 19, 2025 19:36

eduardiazf force-pushed the feat/initial-client branch from de831c0 to acb4938 Compare August 19, 2025 17:36

eduardiazf requested a review from cwasicki August 19, 2025 17:37

cwasicki reviewed Aug 19, 2025

View reviewed changes

eduardiazf enabled auto-merge August 20, 2025 10:01

llucax approved these changes Aug 20, 2025

View reviewed changes

eduardiazf disabled auto-merge August 20, 2025 12:34

eduardiazf merged commit 066a8bd into frequenz-floss:v0.x.x Aug 20, 2025
14 checks passed

llucax mentioned this pull request Aug 20, 2025

Initial client #7

Closed

Implement initial Assets API client with CLI support #21

Implement initial Assets API client with CLI support #21

Uh oh!

Conversation

eduardiazf commented Aug 15, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Summary

Changes Made

🚀 New Features

🔧 Dependencies & Configuration

🧪 Testing Infrastructure

Technical Details

Client Architecture

CLI Features

Usage Examples

Testing

Breaking Changes

Impact

Uh oh!

llucax left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

llucax commented Aug 19, 2025

Uh oh!

llucax left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

llucax commented Aug 19, 2025

Uh oh!

llucax commented Aug 19, 2025

Uh oh!

eduardiazf commented Aug 19, 2025

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

cwasicki left a comment

Choose a reason for hiding this comment

Uh oh!

llucax commented Aug 20, 2025

Uh oh!

llucax commented Aug 20, 2025

Uh oh!

Uh oh!

Uh oh!

eduardiazf commented Aug 15, 2025 •

edited

Loading