Skip to content

obtain a dedicated API key for testing NREL psm3 #870

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

Closed
mikofski opened this issue Jan 25, 2020 · 7 comments · Fixed by #873
Closed

obtain a dedicated API key for testing NREL psm3 #870

mikofski opened this issue Jan 25, 2020 · 7 comments · Fixed by #873
Assignees
Labels
solarfx2 DOE SETO Solar Forecasting 2 / Solar Forecast Arbiter testing
Milestone

Comments

@mikofski
Copy link
Member

Is your feature request related to a problem? Please describe.
came up in #866 see this comment but related to most PR and testing in general - due to NREL PSM3 throttle limits for the DEMO key the CI's frequently fail

Describe the solution you'd like
obtain a dedicated API key from NREL developer network and upload it to Azure pipelines as a secret variable, ditto for Travis, then for testing, get this from os.environ[<PVLIB TEST NREL API KEY>] and use it instead of the DEMO key which has severe throttle limits and is shared by everyone

Describe alternatives you've considered
just live with the test failures, or skip theses tests from the CI's

Additional context
Add any other context or screenshots about the feature request here.

@CameronTStark CameronTStark self-assigned this Jan 27, 2020
@CameronTStark
Copy link
Contributor

I'm up for taking care of this. @wholmgren is there a particular email address or convention that you keep parent accounts for pvlib-python? Otherwise I'll just use my own for this.

@wholmgren
Copy link
Member

Thanks @CameronTStark. We don't have an email address for this kind of thing, but we could create one. @lboeman is it trivial to make an email address like [email protected] that we could use for this purpose? This supports the Arbiter given NREL's interest in using their API for forecast validation.

@lboeman
Copy link
Contributor

lboeman commented Jan 27, 2020

@wholmgren Yes, I went ahead and set that up. You should receive a test email at that address.

@wholmgren wholmgren added this to the 0.7.2 milestone Jan 27, 2020
@wholmgren wholmgren added solarfx2 DOE SETO Solar Forecasting 2 / Solar Forecast Arbiter testing labels Jan 27, 2020
@CameronTStark
Copy link
Contributor

CameronTStark commented Jan 27, 2020

So I've tired this out on my fork and got it working in a test build. The thing that I'm finding is that there seems to be a limited number of API requests we can do per some amount of time. If I run all the tests in test_psm3.py often the second one fails but if I run them individually they pass.

For a test, I ran test_psm3.py through the CI with a single bare_linux build with 3.5, 3.6, 3.7 Python versions and only the 3.5 passed. I chose "rerun failed tests" and the 3.6 and 3.7 tests passed without me changing anything. The error I'm getting is:

"OVER_RATE_LIMIT","You have exceeded your rate limit. Try again later or contact us at https://developer.nrel.gov/contact/ for assistance"

This is the same error as before with just using DEMO_KEY I believe but other tests pass confirming that the API key is configured correctly.

@kanderso-nrel or @mikofski do you know if there's a requests/minute limit for the NREL API?

@kandersolar
Copy link
Member

@CameronTStark The documentation does include rate-limiting, see here: https://developer.nrel.gov/docs/solar/nsrdb/psm3_data_download/#rate-limits

The first limit determines how many requests a given user can make per 24 hour period. For requests utilizing the .csv format this rate limit is set at 2000 a day at a frequency of no more than 1 per second.

I've suspected for some time that the rate limits in practice are (much) stricter than what is documented there -- I've had code with explicit 5-second delays between API calls fail after just a few iterations, without having made any calls for several days. It's been on my to-do list to do more formal investigation and bring it up with the maintainers. As far as I know, there's not necessarily a way to avoid what you're seeing. If others know different, I'd love to be wrong about that!

@CameronTStark
Copy link
Contributor

Thank you for the confirmation that I'm not crazy @kanderso-nrel. I took a few iterations to be sure of what I was seeing.

To make the tests past consistently the only solutions that I can propose are to put 5 sec timers in front of every API call and/or cut down the number of API calls to the bare minimum. Both aren't reasons to be excited to be sure. I'm not sure even NREL whitelisting Azure IPs for a higher throughput would be a workable fix. Do you have any other suggestions?

In looking at test_psm3.py some of the tests made me wonder if we were testing pvlib or the API server. Are there any API calls that could be removed to help the situation?

@kandersolar
Copy link
Member

kandersolar commented Jan 27, 2020

Now that you mention it, this pattern could well be the issue:

    with pytest.raises(HTTPError):
        # HTTP 403 forbidden because api_key is rejected
        psm3.get_psm3(LATITUDE, LONGITUDE, api_key='BAD', email=PVLIB_EMAIL)
    with pytest.raises(HTTPError):
        # coordinates were not found in the NSRDB
        psm3.get_psm3(51, -5, DEMO_KEY, PVLIB_EMAIL)
    with pytest.raises(HTTPError):
        # names is not one of the available options
        psm3.get_psm3(LATITUDE, LONGITUDE, DEMO_KEY, PVLIB_EMAIL, names='bad')

Those could easily send more than one request per second. Maybe we should wrap the get_psm3 function calls with another function that sleeps before every call? For instance

def get_psm3(*args, **kwargs):
    time.sleep(2)
    return psm3.get_psm3(*args, **kwargs)

If that doesn't work, here are two ideas, neither of which is nearly as good as understanding the API rate limits:


The API response includes two relevant headers indicating rate limit usage:

In [124]: dict(response.headers)
Out[124]: 
 ...
 'X-RateLimit-Limit': '2000',
 'X-RateLimit-Remaining': '1999',
 ...

Maybe useful for e.g. monkey-patching the network calls in test_psm3.py to print out those header values:

import requests
import sys
original_get = requests.get

def get_and_log(*args, **kwargs):
    response = original_get(*args, **kwargs)
    headers = dict(response.headers)
    for key in ['X-RateLimit-Limit', 'X-RateLimit-Remaining']:
        print(key, headers.get(key, None))
    return response

psm3.requests.get = get_and_log
Click to expand pytest output
(base) C:\Users\KANDERSO\projects\pvlib-python>pytest -s pvlib\test\test_psm3.py
================================================= test session starts =================================================
platform win32 -- Python 3.7.3, pytest-5.0.1, py-1.8.0, pluggy-0.12.0
rootdir: C:\Users\KANDERSO\projects\pvlib-python
plugins: celery-4.3.0, dash-1.6.0, arraydiff-0.3, cov-2.8.1, doctestplus-0.3.0, openfiles-0.3.2, remotedata-0.3.1
collected 4 items

pvlib\test\test_psm3.py X-RateLimit-Limit 40
X-RateLimit-Remaining 22
X-RateLimit-Limit None
X-RateLimit-Remaining None
X-RateLimit-Limit 40
X-RateLimit-Remaining 21
X-RateLimit-Limit 40
X-RateLimit-Remaining 20
.X-RateLimit-Limit 40
X-RateLimit-Remaining 19
...

I've used a retry approach like this before when I wanted to bulk download data from the API and kept getting seemingly spurious timeouts:

def retry(n=10, delay=5):
    def decorator(func):
        def wrapper(*args, **kwargs):
            for i in range(n):
                try:
                    result = func(*args, **kwargs)
                    break
                except HTTPError:
                    time.sleep(delay)
            else:
                raise Exception('Max retries reached')
            return result
        return wrapper
    return decorator

@retry(n=3, delay=10)
def fetch(lat, lon, year):
    headers, df = pvlib.iotools.get_psm3(lat, lon, key, email, year, 30)
    return df

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
solarfx2 DOE SETO Solar Forecasting 2 / Solar Forecast Arbiter testing
Projects
None yet
Development

Successfully merging a pull request may close this issue.

5 participants