Skip to content

Conversation

terwey
Copy link
Contributor

@terwey terwey commented Sep 18, 2025

feat(core): propagate context through HTTP; add WithContext APIs; fix OID typing & docs

  • Pass context.Context through HTTP:

    • client.post(ctx, ...)
    • exchange.executeAction(ctx, ...), exchange.postAction(ctx, ...)
    • info.postTimeRangeRequest(ctx, ...)
  • Add ...WithContext(ctx, ...) variants across Exchange and Info (orders, cancels, transfers, staking, deploy, validator ops, queries, snapshots, etc.). Legacy methods remain and delegate to context.Background().

…ix OID typing & docs

* Pass `context.Context` through HTTP:
  * `client.post(ctx, ...)`
  * `exchange.executeAction(ctx, ...)`, `exchange.postAction(ctx, ...)`
  * `info.postTimeRangeRequest(ctx, ...)`

* Add `...WithContext(ctx, ...)` variants across `Exchange` and `Info` (orders, cancels, transfers, staking, deploy, validator ops, queries, snapshots, etc.). Legacy methods remain and delegate to `context.Background()`.
@coveralls
Copy link

coveralls commented Sep 18, 2025

Pull Request Test Coverage Report for Build 17829835865

Details

  • 52 of 305 (17.05%) changed or added relevant lines in 6 files are covered.
  • 2 unchanged lines in 1 file lost coverage.
  • Overall coverage decreased (-0.03%) to 16.347%

Changes Missing Coverage Covered Lines Changed/Added Lines %
exchange_orders.go 4 27 14.81%
info.go 35 109 32.11%
exchange_others.go 0 156 0.0%
Files with Coverage Reduction New Missed Lines %
exchange_others.go 2 0.0%
Totals Coverage Status
Change from base Build 17711580431: -0.03%
Covered Lines: 2219
Relevant Lines: 13574

💛 - Coveralls

@sonirico
Copy link
Owner

Appreciate the effort, but I rather push a breaking change adding ctx as 1st param and all methods. If you need it I can do it too.

@terwey
Copy link
Contributor Author

terwey commented Sep 18, 2025

I'd recommend against the breaking change, most people will not necessarily want to use a context at all times anyway.

@sonirico
Copy link
Owner

I'd recommend against the breaking change, most people will not necessarily want to use a context at all times anyway.

Really appreciate your input and concerns. That said, the lib is still at v0.x.x, which gives room for these kinds of changes. The SDK was originally built without context cancellation to stay 100% aligned with the official Python SDK, TBH a decision which I deeply regret from time to time.

If this were an internal repo, I wouldn’t have allowed methods spawning network connections without context in the first place. So I’d rather fix it now than keep carrying that limitation forward.

Also, not that I think too many people are using this repo anyway. Tomorrow I’ll open a PR with this change.

@sonirico
Copy link
Owner

@terwey see #58

@sonirico sonirico closed this Sep 19, 2025
@terwey
Copy link
Contributor Author

terwey commented Sep 19, 2025

@sonirico I missed your comment an hour ago. I'll pull and propose a small change I noticed while doing this work.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants