-
-
Notifications
You must be signed in to change notification settings - Fork 607
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
Drop parfive less than 2 #6942
Drop parfive less than 2 #6942
Conversation
sunpy/conftest.py
Outdated
@pytest.fixture(scope='session', autouse=True) | ||
def parfive_header_test(request): | ||
""" | ||
Add a keyword to tell parfive this is a test run. | ||
""" | ||
os.environ["PARFIVE_SUNPY_TESTS"] = "True" | ||
yield | ||
del os.environ["PARFIVE_SUNPY_TESTS"] |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Was not sure how to do this, so I did this as it was quick.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Seems legit to me 👍
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yeah this seems good, but why don't we make this a generic environment var for being in our test suite? We might have other non-parfive uses for it in the future?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I can do that.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Was not sure what to call it, so I made one up.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
- The
oldestdeps
version pin needs updating intox.ini
- There should be a note in the changelog and in the what's new that says something like "We are aware the change in the parfive minimum version is a release earlier than our dependency policy allows for, but due to significant issues that parfive v2 solves we have decided to bump it now"
- Can you explain a bit more about why you've tweaked the user agent for the tests?
changelog/6742.feature.rst
Outdated
@@ -3,6 +3,7 @@ The minimum required versions of several core dependencies have been updated: | |||
- Python 3.9 | |||
- astropy 5.0.1 | |||
- numpy 1.21.0 | |||
- parfive 2.0.0 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This should go in a new changelog entry, so it's linked with this PR and not the one corresponding to this file.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Fixed.
sunpy/conftest.py
Outdated
@pytest.fixture(scope='session', autouse=True) | ||
def parfive_header_test(request): | ||
""" | ||
Add a keyword to tell parfive this is a test run. | ||
""" | ||
os.environ["PARFIVE_SUNPY_TESTS"] = "True" | ||
yield | ||
del os.environ["PARFIVE_SUNPY_TESTS"] |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Seems legit to me 👍
The motivation is to help the VSO or other providers to be able to see if requests coming from sunpy come either from our CI or actual users. This is used to be more of a problem back in the day when we ran a larger online test suite but I think it would still be nice for data providers to be able to parse logs and know if our test suite is causing a problem or if its users. |
Also a hack for adding the words CI-test to the user agent headers for our tests.
Fixes #6308
Fixes #6940
I adjusted the previous changelog for updated versions instead of a new one.