Skip to content

Conversation

@qdrs
Copy link
Collaborator

@qdrs qdrs commented Dec 4, 2025

No description provided.

@qdrs qdrs force-pushed the feat/markets-request-builder branch from 8d05a8c to 9cc7cb7 Compare December 4, 2025 04:12
Comment on lines +31 to +33
GetMarketsRequest::new()
.with_spot_markets()
.with_perp_markets()
Copy link
Contributor

Choose a reason for hiding this comment

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

As a user I would expect that calling BpxClient::get_markets would be equivalent to invoking https://api.bpx.dev/api/v1/markets.

IMO we should not specify defaults here - the exchange backend already has defaults specified if no types are provided in the query:

let market_types = market_type.unwrap_or_else(|| vec![MarketType::SPOT, MarketType::PERP]);

Comment on lines +112 to +114
pub fn new() -> Self {
Self::default()
}
Copy link
Contributor

Choose a reason for hiding this comment

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

Could we add an additional overload that allows the user to pass a market type? This way if a new market type becomes available in the API it's still possible to query it without us having to add another method here.

}

#[derive(Debug, Default, Clone)]
pub struct GetMarketsRequest(Vec<String>);
Copy link
Contributor

Choose a reason for hiding this comment

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

Not that performance really matters here, but could use Cow<'static, str> here since the market types are mostly going to be static str.


fn validate(&self) -> Result<()>;

fn send(self, client: &BpxClient) -> impl Future<Output = Result<Response>>
Copy link
Contributor

@extremeandy extremeandy Dec 4, 2025

Choose a reason for hiding this comment

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

Could we have a method on BpxClient like:

pub async fn send<R>(&self, request: R) -> impl Future<Output = Result<R::Response>> where R: BpxClientRequest

Then users can do:

let req = GetMarketsRequest::new();
let response = client.send(req).await?;

Feels a little more ergonomic than req.send(client).await

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