Describe the bug
Failures of network dependent iotools tests due to failures of the API server receiving the request affect consistency and confidence in our CI test results.
To Reproduce
Expected behavior
There are two important functionalities that are desirable:
Additional context
This was originally identified by @wholmgren in https://github.com/pvlib/pvlib-python/pull/897#issuecomment-590352000_
Proposed solution
Implement @pytest.mark.flaky()for all remote API dependent tests to reattempt if API is temporarily unreachable. This is already done for tests in test_psm3.py decorated with @pytest.mark.remote_data.
I'm going to look for a pytest functionality to enable a conditional skip of the test in the event the API server is completely unavailable during the test duration. If you have a suggestion on how to accomplish this please let me know.
@pytest.mark.skipif() should work well for this application but the trick will be the implementation for each network dependent API.
It might be ideal that this PR is merged as is after some final polish. The implementation of @pytest.mark.flaky() currently in this PR can be a quick win for 0.7.2 in terms of fixing sporadic dropouts in server dropouts during tests.
The @pytest.mark.flaky() or mock solutions would take (me) significantly more time to figure out and technically solves a separate issue of complete dropout of the external API server.
What do you think @wholmgren, @mikofski, @cwhanse?
It's fine, but pvlib was not the only one affected by NREL's PSM3 failure. I think we need to coordinate with NREL to get some public accounting for what happened.
Ideally, a failure due to API server failure would be repeated with the hope that communication can be established... The implementation of @pytest.mark.flaky() currently in this PR can be a quick win for 0.7.2 in terms of fixing sporadic dropouts in server dropouts during tests.
I agree.
In the event that communication cannot be established during the test, it should be conditionally skipped with a warning that the test wasn't properly run.
I disagree. If GitHub shows all green then I don't look at the CI results, so to me it defeats the purpose of running the test at all.
I'm all for using mock to further minimize the reliance on the network code, but that will take more effort to do right. We should start by ensuring that all of the API-testing code is decoupled from the parser-testing code (as some recent PRs have addressed).
Most helpful comment
I agree.
I disagree. If GitHub shows all green then I don't look at the CI results, so to me it defeats the purpose of running the test at all.
I'm all for using
mockto further minimize the reliance on the network code, but that will take more effort to do right. We should start by ensuring that all of the API-testing code is decoupled from the parser-testing code (as some recent PRs have addressed).