feat(spot): add cl_ord_id to create_order and edit_order endpoints#416
feat(spot): add cl_ord_id to create_order and edit_order endpoints#416claudiapacini wants to merge 1 commit intobtschwertfeger:masterfrom
Conversation
btschwertfeger
added
Should
btschwertfeger
left a comment
btschwertfeger
left a comment
There was a problem hiding this comment.
Hey @claudiapacini, thanks for bringing this up!
Initially, I wanted to reject your issue and PR as the idea does not align with the decision on discontinuing the specialized Market, User, Trade, etc clients as stated here. The recommended functions to achieve your goal (e.g. for Spot trading) are the SpotClient.request and SpotAsyncClient.request methods. It also introduces a breaking change to the amend_order function that relied on the extra_params usage and now requires setting txid or cl_ord_id.
But after going through your changes, I see the benefit in applying them especially considering that the cancel_order function currently does not support client order IDs. While I still don't have the capacity to continuously track Kraken API changes, it makes sense to keep these specialized clients reasonably up to date with the help of contributors like you. This is particularly important not only from a functional perspective, but also because many users are likely still relying on the legacy client interfaces. Since the parameters are mostly optional, the risk of introducing future breaking changes also remains relatively low. So I'm happy to accept your proposed changes once you address my remaining comments.
| def amend_order( # pylint: disable=too-many-arguments # noqa: PLR0913, PLR0917 | ||
| self: Trade, | ||
| txid: str | None = None, | ||
| cl_ord_id: str | None = None, | ||
| order_qty: str | float | None = None, | ||
| display_qty: str | float | None = None, | ||
| limit_price: str | float | None = None, | ||
| trigger_price: str | float | None = None, | ||
| post_only: bool | None = None, | ||
| userref: int | None = None, | ||
| validate: bool = False, |
There was a problem hiding this comment.
Some parameters from the list are still missing https://docs.kraken.com/api/docs/rest-api/amend-order/.
| with pytest.raises( | ||
| KrakenPermissionDeniedError, | ||
| match=r"API key doesn't have permission to make this request.", | ||
| ): | ||
| spot_auth_trade.edit_order( | ||
| txid=self.TEST_TXID, | ||
| cl_ord_id=self.TEST_CL_ORD_ID, | ||
| volume=1.25, | ||
| pair=self.TEST_PAIR_XBTUSD, | ||
| price=27500, | ||
| price2=26500, | ||
| cancel_response=False, | ||
| truncate=True, | ||
| oflags=["post"], | ||
| validate=True, # important | ||
| ) |
There was a problem hiding this comment.
I guess txid should not be part of the parameters here?
| with pytest.raises( | ||
| KrakenPermissionDeniedError, | ||
| match=r"API key doesn't have permission to make this request.", | ||
| ): | ||
| spot_auth_trade.create_order( | ||
| ordertype="limit", | ||
| side="buy", | ||
| volume=10000000, | ||
| pair=self.TEST_PAIR_BTCEUR, | ||
| price=100, | ||
| expiretm="0", | ||
| cl_ord_id="12345", | ||
| validate=True, # important to just test this endpoint without risking money | ||
| ) |
There was a problem hiding this comment.
Lets remove this call and add the cl_ord_id param to one of the calls before. This way we reduce the number of API calls without sacrificing functionality.
2 participants
Summary
Adds the cl_ord_id for create order and edit order endpoints as per the docs:
Trade endpoints
https://docs.kraken.com/api/docs/rest-api/add-order
https://docs.kraken.com/api/docs/rest-api/amend-order
https://docs.kraken.com/api/docs/rest-api/cancel-order
User endpoints
https://docs.kraken.com/api/docs/rest-api/get-open-orders
https://docs.kraken.com/api/docs/rest-api/get-closed-orders
https://docs.kraken.com/api/docs/rest-api/get-orders-info
Closes the open issue:
python-kraken-sdk/issues/415.