Adding support for crypto streaming

[yiftachn]

Hey!
from what I've tried the repo does not support crypto streaming, that is due to a different path structure from the regular streaming api.
I think it's very important, because you can trade crypto 24/7 so you don't need to wait for market open times to test your code.
I'm new to rust, but I think I know how to change that, and if I'll get some timely support I can work on a PR for that.
basically start by making the Source trait return the URL instead of the STR, than incorporate that to ApiInfo.

[d-e-s-o]

Please see #26. I haven't looked into crypto support much.

[d-e-s-o]

If you want to give it a shot, be my guest. I suppose what you are saying is that Source would need to contain a URL to make it work? Not sure why that is -- can you explain? Wouldn't it be sufficient to extend only ApiInfo?

[yiftachn]

This is the relevant code from stream.rs

 let ApiInfo {
      data_stream_base_url: url,
      key_id,
      secret,
      ..
    } = api_info;

    let mut url = url.clone();
    url.set_path(&format!("v2/{}", S::as_str()));

I think that manipulating the url inside stream.rs/subscribe is the cause of the problem to easily add / manipulate sources.
If all the logic regarding the URL will be encapsulated inside ApiInfo, then it will be much more flexible and adding crypto will be easy.
There are a few options I can think of:

• the best thing to do IMO is delete the Source and let the user input the specific URL he wishes - be it iex sip or whatever.

• another option is to make each source implement his own "get url". then the user chooses which source to use and everything he needs is taken care of.

what do you think?

[d-e-s-o]

If all the logic regarding the URL will be encapsulated inside ApiInfo, then it will be much more flexible and adding crypto will be easy. There are a few options I can think of:

• the best thing to do IMO is delete the Source and let the user input the specific URL he wishes - be it iex sip or whatever.

That will be less user-friendly, though, because now users have to fiddle around with stream URL endpoints. That will probably result in a messy situation, like here: #51 (comment)
It's also not really the level of abstraction that this crate aims to provide.

Crypto streaming is all beta stuff from what I understand. I am reluctant to include anything beta in this crate -- Alpaca can't even provide stable "stable" APIs, so I am not putting up with anything openly declared unstable. That's also the reason why there is no news support, for example.

So I don't think crypto will end up receiving first class support. That being said, I think one avenue we have is to go with a mixture of what you propose:

• ApiInfo stays the same
• we introduce another (perhaps hidden) type impl'ing Source and that can accept a user provided URL (e.g., as part of its constructor)
• then, conceptually, users can do client.subscribe::<RealtimeData<UserUrl>>().await and it would work

Of course I papered over a bunch of implementation details here that I haven't thought through in detail. Specifically, as you said currently Source has an as_str method only. We'd need a design that can convey to the caller (i.e., RealtimeData::connect) that what is being returned is a fully blown URL, not just part of it. Could be an enum or whatever. So then RealtimeData::connect would either construct the URL (what we currently do) or just take the one from UserUrl (to stick with the example naming). The main problem will be that currently the URL would have to be a compile time constant, because we don't pass in a Source instance, only the type info. These are all "internal" APIs basically, so it should be possible to get this massaged accordingly, though. I hope this makes sense.

[yiftachn]

The idea meks sense and I understand why you prefer it that way.
But in any case I think the URL shouldn't be manipulated inside the connect function, so I think a redesign where connect gets the URL from api info which gets the string from source is best.
Adding that Enum is a patch that adds clutter IMO.
If you agree I can start.

But I just to say that I think it's important crypto will get a first class support as it makes developing much faster - because it's streaming 24/7.

I for example don't plan to trade crypto but I want to develop using crypto then switch and test on stocks.

For me waiting hours to test my program is unacceptable, that's why I offered to PR. I guess for others too.

If you agree, than let's do the second option and have a Crypto source, and make everything work seamlessly from there.

I can do them both you choose

[d-e-s-o]

But in any case I think the URL shouldn't be manipulated inside the connect function, so I think a redesign where connect gets the URL from api info which gets the string from source is best.

If the URL is in the ApiInfo object, we can't subscribe to different streams at runtime, can we? So that's an unnecessary restriction. On top of that we now force users to pick the entire URL, which I already explained is not great. So I don't see how just going with additional info in the ApiInfo type we can get sufficiently far without regressing on what we have.

[d-e-s-o]

If you agree, than let's do the second option and have a Crypto source, and make everything work seamlessly from there.

And if we do that we are back at enabling some beta stuff, right, which I mentioned is something I'd rather not do :-|