Skip to content
Merged
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: 2 additions & 0 deletions RELEASE_NOTES.md
Original file line number Diff line number Diff line change
Expand Up @@ -16,4 +16,6 @@

## Bug Fixes

* Deal correctly with cancelled orders with no price or quantity

<!-- Here goes notable bug fixes that are worth a special mention or explanation -->
30 changes: 27 additions & 3 deletions src/frequenz/client/electricity_trading/_types.py
Original file line number Diff line number Diff line change
Expand Up @@ -1010,8 +1010,16 @@ def from_pb(cls, order: electricity_trading_pb2.Order) -> Self:
delivery_period=DeliveryPeriod.from_pb(order.delivery_period),
type=OrderType.from_pb(order.type),
side=MarketSide.from_pb(order.side),
price=Price.from_pb(order.price),
quantity=Power.from_pb(order.quantity),
price=(
Price.from_pb(order.price)
if order.HasField("price")
else Price(Decimal("NaN"), Currency.UNSPECIFIED)
),
quantity=(
Power.from_pb(order.quantity)
if order.HasField("quantity")
else Power(Decimal("NaN"))
),
stop_price=(
Price.from_pb(order.stop_price)
if order.HasField("stop_price")
Expand Down Expand Up @@ -1308,8 +1316,11 @@ 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.
"""
return cls(
od = cls(
order_id=order_detail.order_id,
order=Order.from_pb(order_detail.order),
state_detail=StateDetail.from_pb(order_detail.state_detail),
Expand All @@ -1321,6 +1332,19 @@ def from_pb(cls, order_detail: electricity_trading_pb2.OrderDetail) -> Self:
),
)

# 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}`)."
)

return od

def to_pb(self) -> electricity_trading_pb2.OrderDetail:
"""Convert an OrderDetail object to protobuf OrderDetail.

Expand Down
54 changes: 27 additions & 27 deletions src/frequenz/client/electricity_trading/cli/etrading.py
Original file line number Diff line number Diff line change
Expand Up @@ -97,7 +97,7 @@ async def list_orders(
"""
client = Client(server_url=url, auth_key=key)

# print_header()
print_order_header()

delivery_period = None
# If delivery period is selected, list historical orders also
Expand Down Expand Up @@ -200,18 +200,18 @@ async def cancel_order(
def print_trade_header() -> None:
"""Print trade header in CSV format."""
header = (
"public_trade_id, "
"execution_time, "
"delivery_period_start, "
"delivery_period_duration, "
"buy_delivery_area_code, "
"sell_delivery_area_code, "
"buy_delivery_area_code_type, "
"sell_delivery_area_code_type"
"quantity_mw, "
"price, "
"currency, "
"state, "
"public_trade_id,"
"execution_time,"
"delivery_period_start,"
"delivery_period_duration,"
"buy_delivery_area_code,"
"sell_delivery_area_code,"
"buy_delivery_area_code_type,"
"sell_delivery_area_code_type,"
"quantity_mw,"
"currency,"
"price,"
"state "
)
print(header)

Expand All @@ -238,20 +238,20 @@ def print_trade(trade: PublicTrade) -> None:
def print_order_header() -> None:
"""Print order header in CSV format."""
header = (
"order_id, "
"create_time, "
"modification_time, "
"delivery_period_start, "
"delivery_period_duration"
"delivery_area_code, "
"delivery_area_code_type, "
"order_type, "
"quantity_mw, "
"open_quantity_mw, "
"side, "
"currency, "
"price, "
"state, "
"order_id,"
"create_time,"
"modification_time,"
"delivery_period_start,"
"delivery_period_duration,"
"delivery_area_code,"
"delivery_area_code_type,"
"order_type,"
"quantity_mw,"
"open_quantity_mw,"
"side,"
"currency,"
"price,"
"state"
)
print(header)

Expand Down
23 changes: 23 additions & 0 deletions tests/test_types.py
Original file line number Diff line number Diff line change
Expand Up @@ -498,6 +498,29 @@ 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."""
# Missing price
od_pb1 = electricity_trading_pb2.OrderDetail()
od_pb1.CopyFrom(ORDER_DETAIL_PB)
od_pb1.order.ClearField("price")
# Not allowed for active orders
with pytest.raises(ValueError):
OrderDetail.from_pb(od_pb1)
# But allowed for canceled orders
od_pb1.state_detail.state = electricity_trading_pb2.OrderState.ORDER_STATE_CANCELED
OrderDetail.from_pb(od_pb1)

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


def test_order_detail_no_timezone_error() -> None:
"""Test that an order detail with inputs with no timezone raises a ValueError."""
with pytest.raises(ValueError):
Expand Down
Loading