Skip to content

Add quote streaming #108

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
bguban wants to merge 3 commits into
base: master
Choose a base branch
from
Open

Add quote streaming #108

Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
17c2b0d
Added base functionality
bguban Sep 21, 2021
8776ee5
fixes
bguban Sep 22, 2021
88142b8
fix README.md
bguban Sep 22, 2021
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 2 additions & 0 deletions lib/iex/api/config/client.rb
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,7 @@ module Client
ca_file
ca_path
endpoint
sse_endpoint
open_timeout
proxy
publishable_token
Expand All @@ -24,6 +25,7 @@ def reset!
self.ca_file = defined?(OpenSSL) ? OpenSSL::X509::DEFAULT_CERT_FILE : nil
self.ca_path = defined?(OpenSSL) ? OpenSSL::X509::DEFAULT_CERT_DIR : nil
self.endpoint = 'https://cloud.iexapis.com/v1'
Copy link
Owner

@dblock dblock Sep 21, 2021

Choose a reason for hiding this comment

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

I don’t love that we’re adding another parameter here. We’re modifying the endpoint, not adding another one. Maybe because there’s just one API that goes to a different endpoint, hardcore that in the request and allow me to overwrite it if I want to.

Copy link
Collaborator Author

@bguban bguban Sep 22, 2021

Choose a reason for hiding this comment

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

I decided to use endpoint.gsub('://cloud.', '://cloud-sse.') to generate the new endpoint. It allows users to specify the API version in the endpoint config.

Copy link
Owner

@dblock dblock Sep 22, 2021 •
edited
Loading

Choose a reason for hiding this comment

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

This feels like the least future proof option of all :)

I was thinking that stream_quote would do this:

def stream_quote(symbols, options = {})
        options = { token: secret_token, endpoint: '...cloud-sse...' }.merge(options)  
        options[:symbols] = Array(symbols).join(',')
        interval = options.delete(:interval)

        get_stream("stocksUS#{interval}", options) do |payload|
          payload.each do |quote|
            yield IEX::Resources::Quote.new(quote)
          end
        end

This way if that endpoint does change, one can override it in the request without releasing a new version of the library.

Copy link
Collaborator Author

@bguban bguban Sep 22, 2021

Choose a reason for hiding this comment

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

Oh, I missed your comment. IEX supports a lot of other streaming endpoints. For example, https://iexcloud.io/docs/api/#cryptocurrency is for streaming crypto. I don't think that it's a good idea to hard code it in every method that will use streaming.
Again, as I understand we have this endpoint configuration to give the ability to use a sandbox or specify a specific API version, but if we hard code that then it will require people to set sandbox API in the code instead of the configuration.
I checked the sandbox documentation https://iexcloud.io/docs/api/#testing-sandbox and it looks like my current solution will not work with the sandbox env at all. So I think that the first implementation (where we had an extra config option) is the best. It allows us to specify different URLs for different environments, it's straight and simple. And if IEX decides to change the URL (which I doubt very much) then developers will be required just to update their config.

Copy link
Collaborator Author

@bguban bguban Sep 28, 2021

Choose a reason for hiding this comment

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

@dblock so do you want me to hard code the URL or is it still an open discussion? if we are going to hard code then what do you suggest to do with a sandbox URL?

Copy link
Owner

@dblock dblock Sep 28, 2021

Choose a reason for hiding this comment

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

Ok, I think you're right. SSE is a thing on its own.

I think we should build SSE support next to the current endpoint implementation rather than attempting to bolt it on top. This means namespacing SSE support under IEX::Api::Streaming or IEX::Streaming::API or IEX:SSE::API - no strong preference on which one.

SSE would have its own implementation, its own connection/request/etc. Much of it can be shared from the existing implementation, meaning you'll want to extract some base classes and potentially move IEX::Api into IEX::Web::Api or IEX::Rest::Api.

This is what I think we want to write:

IEX::Streaming::Api.configure do |config|
  config.endpoint = 'https://.... sse
end

IEX::Streaming::Client.new(...)

Copy link
Collaborator Author

@bguban bguban Sep 30, 2021

Choose a reason for hiding this comment

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

I'm not sure that it's worth the effort:

  1. it will require us to make a major release because we change the namespace and every developer must update their code
  2. we will decrease readability when trying to share the functionality. there will be a lot of super calls with small updates.

And honestly say I don't see any benefits from separating the functionality to different namespaces. The only difference in the code is request.rb file that requires this event parser. So maybe it makes sense to create ansse_request.rb file that will be responsible for streaming (or just continue using the request.rb). Or we can try to implement a Faraday middleware that will take event parser responsibility.

Probably it makes sense to ask other contributors their opinions.

Copy link
Owner

@dblock dblock Sep 30, 2021

Choose a reason for hiding this comment

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

I just think it would make things a lot cleaner and I expect IEX to keep adding endpoints. If you don't feel like doing it, I might pick it up.

self.sse_endpoint = 'https://cloud-sse.iexapis.com/v1'
self.publishable_token = ENV['IEX_API_PUBLISHABLE_TOKEN']
self.secret_token = ENV['IEX_API_SECRET_TOKEN']
self.user_agent = "IEX Ruby Client/#{IEX::VERSION}"
Expand Down
19 changes: 17 additions & 2 deletions lib/iex/cloud/request.rb
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,10 +1,25 @@
module IEX
module Cloud
module Request
STREAM_EVENT_DELIMITER = "\r\n\r\n".freeze

def get(path, options = {})
request(:get, path, options)
end

def get_stream(path, options = {})
buffer = ""
event_parser = Proc.new do |chunk|
events = (buffer + chunk).lines(STREAM_EVENT_DELIMITER)
buffer = events.last.end_with?(STREAM_EVENT_DELIMITER) ? '' : events.delete_at(-1)
events.each do |event|
yield JSON.parse(event.gsub(/\Adata: /, ''))
end
end

request(:get, path, { endpoint: sse_endpoint, request: { on_data: event_parser } }.merge(options))
end

def post(path, options = {})
request(:post, path, options)
end
Expand All @@ -20,16 +35,16 @@ def delete(path, options = {})
private

def request(method, path, options)
path = [endpoint, path].join('/')
path = [options.delete(:endpoint) || endpoint, path].join('/')
response = connection.send(method) do |request|
request.options.merge!(options.delete(:request)) if options.key?(:request)
case method
when :get, :delete
request.url(path, options)
when :post, :put
request.path = path
request.body = options.to_json unless options.empty?
end
request.options.merge!(options.delete(:request)) if options.key?(:request)
end
response.body
end
Expand Down
13 changes: 13 additions & 0 deletions lib/iex/endpoints/quote.rb
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,19 @@ def quote(symbol, options = {})
rescue Faraday::ResourceNotFound => e
raise IEX::Errors::SymbolNotFoundError.new(symbol, e.response[:body])
end

# @param symbols - a list of symbols
# @param options[:interval] sets intervals such as 1Second, 5Second, or 1Minute
def stream_quote(symbols, options = {})
options[:symbols] = Array(symbols).join(',')
interval = options.delete(:interval)

get_stream("stocksUS#{interval}", { token: secret_token }.merge(options)) do |payload|
payload.each do |quote|
yield IEX::Resources::Quote.new(quote)
end
end
end
end
end
end
61 changes: 61 additions & 0 deletions spec/fixtures/iex/stream_quote/spy.yml
View file
Open in desktop

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

73 changes: 46 additions & 27 deletions spec/iex/endpoints/quote_spec.rb
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2,39 +2,58 @@

describe IEX::Resources::Quote do
include_context 'client'
context 'known symbol', vcr: { cassette_name: 'quote/msft' } do
subject do
client.quote('MSFT')
end
it 'retrieves a quote' do
expect(subject.symbol).to eq 'MSFT'
expect(subject.company_name).to eq 'Microsoft Corp.'
expect(subject.market_cap).to eq 915_754_985_600
end
it 'coerces numbers' do
expect(subject.latest_price).to eq 119.36
expect(subject.change).to eq(-0.61)
expect(subject.week_52_high).to eq 120.82
expect(subject.week_52_low).to eq 87.73
expect(subject.change_percent).to eq(-0.00508)
expect(subject.change_percent_s).to eq '-0.51%'
expect(subject.extended_change_percent).to eq(-0.00008)
expect(subject.extended_change_percent_s).to eq '-0.01%'

describe '#quote' do
context 'known symbol', vcr: { cassette_name: 'quote/msft' } do
subject do
client.quote('MSFT')
end
it 'retrieves a quote' do
expect(subject.symbol).to eq 'MSFT'
expect(subject.company_name).to eq 'Microsoft Corp.'
expect(subject.market_cap).to eq 915_754_985_600
end
it 'coerces numbers' do
expect(subject.latest_price).to eq 119.36
expect(subject.change).to eq(-0.61)
expect(subject.week_52_high).to eq 120.82
expect(subject.week_52_low).to eq 87.73
expect(subject.change_percent).to eq(-0.00508)
expect(subject.change_percent_s).to eq '-0.51%'
expect(subject.extended_change_percent).to eq(-0.00008)
expect(subject.extended_change_percent_s).to eq '-0.01%'
end
it 'coerces times' do
expect(subject.latest_update).to eq 1_554_408_000_193
expect(subject.latest_update_t).to eq Time.at(1_554_408_000)
expect(subject.iex_last_updated).to eq 1_554_407_999_529
expect(subject.iex_last_updated_t).to eq Time.at(1_554_407_999)
end
end
it 'coerces times' do
expect(subject.latest_update).to eq 1_554_408_000_193
expect(subject.latest_update_t).to eq Time.at(1_554_408_000)
expect(subject.iex_last_updated).to eq 1_554_407_999_529
expect(subject.iex_last_updated_t).to eq Time.at(1_554_407_999)

context 'invalid symbol', vcr: { cassette_name: 'quote/invalid' } do
subject do
client.quote('INVALID')
end
it 'fails with SymbolNotFoundError' do
expect { subject }.to raise_error IEX::Errors::SymbolNotFoundError, 'Symbol INVALID Not Found'
end
end
end

context 'invalid symbol', vcr: { cassette_name: 'quote/invalid' } do
describe '#stream_quote' do
subject do
client.quote('INVALID')
quotes = []
client.stream_quote('SPY', interval: '5Second') do |quote|
quotes << quote
end

quotes.first
end
it 'fails with SymbolNotFoundError' do
expect { subject }.to raise_error IEX::Errors::SymbolNotFoundError, 'Symbol INVALID Not Found'

it 'retrieves a quote', vcr: { cassette_name: 'stream_quote/spy' } do
expect(subject.symbol).to eq('SPY')
expect(subject.close).to eq(433.63)
end
end
end