Skip to content
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.

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

Multiple Stocks fail with alpha_vantage component #22423

Closed
Per4merKC opened this issue Mar 26, 2019 · 19 comments
Closed

Multiple Stocks fail with alpha_vantage component #22423

Per4merKC opened this issue Mar 26, 2019 · 19 comments

Comments

@Per4merKC
Copy link

Home Assistant release with the issue:
0.90.1

Last working Home Assistant release (if known):
unknown

Operating environment (Hass.io/Docker/Windows/etc.):
hassio running on Raspberry Pi

Component/platform:
alpha_vantage

Description of problem:
Several issues have been opened regarding the KeyError: 'Time Series (15min)', startup error and api limit. I read that we could get a premium API key; however, those cost a minimum of $30 USD per month up to $250 USD per month!! There really needs to be some way to do a circular queue "round-robin" of the stocks in the API call to not exceed the value. I think that most of us are not using these stock prices as a real-time indicator to buy/sell stock... we simply want to have a convenient place to see our stock prices.

Problem-relevant configuration.yaml entries and (fill out even if it seems unimportant):

---
platform: alpha_vantage
api_key: !secret alpha_vantage_key
symbols:
  - name: Sprint
    currency: USD
    symbol: S
  - name: Apple
    currency: USD
    symbol: AAPL
  - name: Google
    currency: USD
    symbol: GOOGL

Traceback (if applicable):

Tue Mar 26 2019 09:50:56 GMT-0500 (CDT)

alpha_vantage: Error on device update!
Traceback (most recent call last):
  File "/usr/local/lib/python3.7/site-packages/homeassistant/helpers/entity_platform.py", line 248, in _async_add_entity
    await entity.async_device_update(warning=False)
  File "/usr/local/lib/python3.7/site-packages/homeassistant/helpers/entity.py", line 379, in async_device_update
    await self.hass.async_add_executor_job(self.update)
  File "/usr/local/lib/python3.7/concurrent/futures/thread.py", line 57, in run
    result = self.fn(*self.args, **self.kwargs)
  File "/usr/local/lib/python3.7/site-packages/homeassistant/components/sensor/alpha_vantage.py", line 161, in update
    all_values, _ = self._timeseries.get_intraday(self._symbol)
  File "/usr/local/lib/python3.7/site-packages/alpha_vantage/alphavantage.py", line 178, in _format_wrapper
    data = call_response[data_key]
KeyError: 'Time Series (15min)'

Additional information:
Issues #15271 offered a solution to get multiple stocks at the same time thus reducing the number of API calls which would also help this situation.

Without implementing something to regulate the API calls, anyone with more than one stock to monitor cannot use this component.

@rohankapoorcom
Copy link
Member

I'm going to take a look and see what I can do to pull the data update out to a single call.

@rohankapoorcom rohankapoorcom self-assigned this Mar 30, 2019
@rohankapoorcom
Copy link
Member

@Per4merKC I've been looking into the batch API and it looks like the batch API will only provide the following information per stock:

{'1. symbol': 'GOOG', '2. price': '1172.9400', '3. volume': '--', '4. timestamp': '2019-03-29 15:59:59'}

This means we would lose the fidelity of the open/close/high/low by switching to the batch API. We could move to doing that by default and then adding an option to enable the updates for open/close/high/low by individual API calls (current functionality).

@Per4merKC
Copy link
Author

@rohankapoorcom Thanks for the information. I'm not sure I understand how that would work. Is a better solution, to switch from all stocks pulling the data at once, to pull them one at a time, oldest stock update first? That would give all of the data we currently have, it would just not be updated as often.

@rohankapoorcom
Copy link
Member

@Per4merKC Unfortunately with the free API we're still limited to 5 API calls per minute and 500 per day.

What we could do is take the number of API calls and divide that by the number of stock tickers provided to figure out the maximum number of updates/day and then work from there. We could then stagger the updates to be equally spaced out as well.

For example, suppose I had 5 stock tickers. That means I can update each stock ticker 100 times per day (per the API limits). Since there's 86,400 seconds in a day, that means that I could get updates no more than once every 864 seconds or about once every 14.4 minutes. Since the API allows for 5 calls in a minute and I'd be tracking 5 stocks, all calls be done within the same minute so all updates happened at once.

Let's look at a second example though with 10 stock tickers. In this case, the API would allow me to update each stock 50 times per day. This means that I can get updates no more than once every 1728 seconds or about once every 28.8 minutes. We would also have to stagger the update calls so that the first five stocks were updated in the first minute and then we waited a couple of minutes before making the next set of updates.

As someone who doesn't use this component, I'm curious if this approach makes sense or whether the data updates would be too slow?

@rohankapoorcom
Copy link
Member

@Per4merKC I opened a WIP PR (#22605) which centralizes the API calls per my first comment (losing the high/low/open/close). Feel free to try that out as a custom component and provide some feedback.

@Per4merKC
Copy link
Author

Thanks, @rohankapoorcom! I can't try that until Wednesday due to my schedule. I'll let you know what I find.

@Per4merKC
Copy link
Author

@rohankapoorcom I tried the updated files as a custom_component, but it fails with the following:

Error during setup of component alpha_vantage
Traceback (most recent call last):
  File "/usr/local/lib/python3.7/site-packages/homeassistant/setup.py", line 154, in _async_setup_component
    component.setup, hass, processed_config)  # type: ignore
  File "/usr/local/lib/python3.7/concurrent/futures/thread.py", line 57, in run
    result = self.fn(*self.args, **self.kwargs)
  File "/config/custom_components/alpha_vantage/__init__.py", line 62, in setup
    conf = config[DOMAIN]
KeyError: 'alpha_vantage'

Does that mean anything to you?

@rohankapoorcom
Copy link
Member

@Per4merKC Unfortunately, there is a breaking change in how the component is configured:

Note that there is no longer any sensor, but alpha_vantage is at the top level now.

alpha_vantage:
  api_key: API_KEY_GOES_HERE
  symbols:
    - symbol: GOOG
      name: Google
    - symbol: AAPL
      name: Apple
  foreign_exchange:
    - from: USD
      to: SGD

@Per4merKC
Copy link
Author

Okay... I'll reconfigure that in the morning and let you know what I find.

@Per4merKC
Copy link
Author

@rohankapoorcom I have your changes implemented as a custom_component and running it with 5 stocks. Tomorrow I will add a few more stocks that I track and let you know if I run into any issues.

Just looking at what is there, I think the high/low/open/close values might be important for us to retain.

@Per4merKC
Copy link
Author

@rohankapoorcom Is there anything new on this issue?

@rohankapoorcom
Copy link
Member

Not yet. Thanks for the feedback. I'll look at this further over the weekend, I've been out of town.

@Per4merKC
Copy link
Author

Just let me know when you would like me to test more. :)

@Per4merKC
Copy link
Author

@rohankapoorcom Any new updates? How can I help?

@Per4merKC
Copy link
Author

@rohankapoorcom Rohan, is there anything I can do to help? I appreciate that you have taken up this task.

@fmonday
Copy link

fmonday commented Jun 14, 2019

Note that there is no longer any sensor, but alpha_vantage is at the top level now.

I can’t figure out how this is meant to work- it still goes under sensor, but without “platform”? Adding it at the root level in configuration.yaml yields “no setup function defined” in the log.

Mine tries to load using the old format, but I’m also getting the Time series (15 min) error

@Per4merKC
Copy link
Author

@rohankapoorcom Is there anything I can do to assist with your development?

@Per4merKC
Copy link
Author

I wish I could figure out how to update this code, but I'm a noob with python. Issue has been open since March.

@stale
Copy link

stale bot commented Nov 11, 2019

There hasn't been any activity on this issue recently. Due to the high number of incoming GitHub notifications, we have to clean some of the old issues, as many of them have already been resolved with the latest updates.
Please make sure to update to the latest Home Assistant version and check if that solves the issue. Let us know if that works for you by adding a comment 👍
This issue now has been marked as stale and will be closed if no further activity occurs. Thank you for your contributions.

@stale stale bot added the stale label Nov 11, 2019
@stale stale bot closed this as completed Nov 18, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging a pull request may close this issue.

4 participants