Skip to content

kraken.spot.trade.create_order: stp_type should be stptype#373

Merged
btschwertfeger merged 3 commits into
btschwertfeger:masterfrom
HFFP:iocUpdate
Apr 11, 2025
Merged

kraken.spot.trade.create_order: stp_type should be stptype#373
btschwertfeger merged 3 commits into
btschwertfeger:masterfrom
HFFP:iocUpdate

Conversation

@HFFP
Copy link
Copy Markdown
Contributor

@HFFP HFFP commented Apr 10, 2025

Fix the stp_type parameter, which must be stptype.

Closes #374

fix: 1. update stp_type to stptype 2. when time in force is IOC, remove starttm
@HFFP HFFP requested a review from btschwertfeger as a code owner April 10, 2025 12:56
@btschwertfeger btschwertfeger added the Bug Something isn't working label Apr 10, 2025
@btschwertfeger btschwertfeger added this to the Upcoming Release milestone Apr 10, 2025
@btschwertfeger
Copy link
Copy Markdown
Owner

Hello @HFFP, I appreciate your time spend on preparing this pull request.

I'm interested in approving your change after addressing the following points:

  • Please create an issue that briefly describes the problem and link it to this pull request.
  • Update the pull request description to not include hints from the pull request template boilerplate that is not related to the change.
  • We don’t need to handle ignoring starttm when timeinforce is IOC within the SDK. Parameter handling was added for historical reasons and made sense back then. But given the current complexity and improved API-side validation, maintaining this logic in the SDK is redundant. Delegating it to the Kraken API is cleaner and more maintainable.

@HFFP
Copy link
Copy Markdown
Contributor Author

HFFP commented Apr 11, 2025

Sure, Sir~

@btschwertfeger btschwertfeger changed the title REST Trade params fix(IOC, stptype) kraken.spot.trade.create_order: stp_type should be stptype Apr 11, 2025
Comment thread src/kraken/spot/trade.py Outdated
@btschwertfeger btschwertfeger self-requested a review April 11, 2025 05:04
@btschwertfeger
Copy link
Copy Markdown
Owner

btschwertfeger commented Apr 11, 2025

Closing and reopening the PR, since somehow the GitHub Actions did not run after the last commit.

@btschwertfeger btschwertfeger enabled auto-merge (squash) April 11, 2025 05:26
@btschwertfeger btschwertfeger merged commit d79af96 into btschwertfeger:master Apr 11, 2025
28 of 38 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Bug Something isn't working

Projects

None yet

Development

Successfully merging this pull request may close these issues.

stp_type need change to stptype

2 participants