Skip to content
Draft
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
2 changes: 1 addition & 1 deletion RELEASE_NOTES.md
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,7 @@

## Upgrading

<!-- Here goes notes on how to upgrade from previous versions, including deprecations and what they should be replaced with -->
* Listing gridpool orders that do not contain a price or quantity no longer raises an exception. Users must validate orders themselves.

## New Features

Expand Down
29 changes: 16 additions & 13 deletions src/frequenz/client/electricity_trading/_types.py
Original file line number Diff line number Diff line change
Expand Up @@ -1316,9 +1316,6 @@ def from_pb(cls, order_detail: electricity_trading_pb2.OrderDetail) -> Self:

Returns:
OrderDetail object corresponding to the protobuf message.

Raises:
ValueError: If the order price and quantity are not specified for a non-canceled order.
"""
Copy link

Copilot AI Aug 15, 2025

Choose a reason for hiding this comment

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

The docstring should be updated to mention that invalid orders are now supported and describe when an order is considered invalid.

Copilot uses AI. Check for mistakes.
od = cls(
order_id=order_detail.order_id,
Expand All @@ -1332,18 +1329,24 @@ def from_pb(cls, order_detail: electricity_trading_pb2.OrderDetail) -> Self:
),
)

return od

@property
def is_valid(self) -> bool:
"""Check if the order detail is valid.
Copy link
Contributor

Choose a reason for hiding this comment

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

The docs should tell what is considered as a valid order.


Returns:
True if the order detail is valid, False otherwise.
"""
# Only cancelled orders are allowed to have missing price or quantity
missing_price_or_quantity = (
od.order.price.amount.is_nan()
or od.order.price.currency == Currency.UNSPECIFIED
or od.order.quantity.mw.is_nan()
)
if missing_price_or_quantity and od.state_detail.state != OrderState.CANCELED:
raise ValueError(
f"Price and quantity must be specified for a non-canceled order (`{od}`)."
)
if self.state_detail.state == OrderState.CANCELED:
return True

return od
return not (
self.order.price.amount.is_nan()
or self.order.price.currency == Currency.UNSPECIFIED
or self.order.quantity.mw.is_nan()
)

def to_pb(self) -> electricity_trading_pb2.OrderDetail:
"""Convert an OrderDetail object to protobuf OrderDetail.
Expand Down
17 changes: 12 additions & 5 deletions tests/test_types.py
Original file line number Diff line number Diff line change
Expand Up @@ -551,25 +551,32 @@ def test_order_detail_from_pb() -> None:

def test_order_detail_from_pb_missing_fields() -> None:
"""Test the client order detail type conversion from protobuf with missing fields."""
# Previously missing price or quantity raised an error, now it is handled gracefully.
# Missing price
od_pb1 = electricity_trading_pb2.OrderDetail()
od_pb1.CopyFrom(ORDER_DETAIL_PB)
OrderDetail.from_pb(od_pb1)
assert OrderDetail.from_pb(od_pb1).is_valid
od_pb1.order.ClearField("price")
OrderDetail.from_pb(od_pb1)
# Not allowed for active orders
with pytest.raises(ValueError):
OrderDetail.from_pb(od_pb1)
# But allowed for canceled orders
assert not OrderDetail.from_pb(od_pb1).is_valid
od_pb1.state_detail.state = electricity_trading_pb2.OrderState.ORDER_STATE_CANCELED
OrderDetail.from_pb(od_pb1)
# But allowed for canceled orders
assert OrderDetail.from_pb(od_pb1).is_valid

# Missing quantity (same logic as above)
od_pb2 = electricity_trading_pb2.OrderDetail()
od_pb2.CopyFrom(ORDER_DETAIL_PB)
OrderDetail.from_pb(od_pb2)
assert OrderDetail.from_pb(od_pb2).is_valid
od_pb2.order.ClearField("quantity")
with pytest.raises(ValueError):
OrderDetail.from_pb(od_pb2)
OrderDetail.from_pb(od_pb2)
assert not OrderDetail.from_pb(od_pb2).is_valid
od_pb2.state_detail.state = electricity_trading_pb2.OrderState.ORDER_STATE_CANCELED
OrderDetail.from_pb(od_pb2)
assert OrderDetail.from_pb(od_pb2).is_valid


def test_order_detail_no_timezone_error() -> None:
Expand Down