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
Update chardet version limit from 5 to 6 #7029
Conversation
Codecov Report
@@ Coverage Diff @@
## maint #7029 +/- ##
==========================================
+ Coverage 75.41% 75.93% +0.51%
==========================================
Files 354 354
Lines 58751 58596 -155
Branches 6613 6610 -3
==========================================
+ Hits 44310 44497 +187
+ Misses 14427 14083 -344
- Partials 14 16 +2
Help us with your feedback. Take ten seconds to tell us how you rate us. Have a feature suggestion? Share it here. |
Let requests maintainers handle the upper limit if necessary.
Code Climate has analyzed commit 0f27cd5 and detected 1 issue on this pull request. Here's the issue category breakdown:
View more on Code Climate. |
This PR somehow failed to retrieve the Appveyor build status and displays a failure, but the actual build succeeded: https://ci.appveyor.com/project/mih/datalad/builds/44759641 |
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 is what we agreed on, so LGTM! Thx!
eh, "we agreed" with reasoning but now I see that travis seems was not even ran , and now we get failed tests like #7049 (comment) on travis only. also nobody requested/created changelog entry before merging. overall: not kosher IMHO. |
This reverts commit 0f27cd5. Original change sounded kosher and "we agreed" with reasoning but original PR datalad#7029 for some reason did not have Travis CI runs and now after it was merged we keep getting tests failed when submitting PRs against maint with failures like ____________________________ test_run_datalad_help _____________________________ [gw0] linux -- Python 3.8.6 /tmp/dl-miniconda-jm48jjoo/bin/python @assert_cwd_unchanged def test_run_datalad_help(): out, err = check_run_and_get_output("datalad --help") ok_startswith(out, "Usage: ") # There could be a warning from coverage that no data was collected, should be benign lines = [l for l in err.split(os.linesep) if ('no-data-collected' not in l) and l] > eq_(lines, []) ../datalad/tests/test_installed.py:47: _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ first = ["/tmp/dl-miniconda-jm48jjoo/lib/python3.8/site-packages/requests/__init__.py:102: RequestsDependencyWarning: urllib3 ...ported version!", ' warnings.warn("urllib3 ({}) or chardet ({})/charset_normalizer ({}) doesn\'t match a supported "'] second = [], msg = None def assert_equal(first, second, msg=None): if msg is None: > assert first == second E assert ['/tmp/dl-min... supported "'] == [] E Left contains 2 more items, first extra item: "/tmp/dl-miniconda-jm48jjoo/lib/python3.8/site-packages/requests/__init__.py:102: RequestsDependencyWarning: urllib3 (1.26.8) or chardet (5.0.0)/charset_normalizer (2.0.4) doesn't match a supported v ersion!" E Full diff: E [ E - , E + '/tmp/dl-miniconda-jm48jjoo/lib/python3.8/site-packages/requests/__init__.py:102: ' E + 'RequestsDependencyWarning: urllib3 (1.26.8) or chardet ' E + "(5.0.0)/charset_normalizer (2.0.4) doesn't match a supported version!", E + ' warnings.warn("urllib3 ({}) or chardet ({})/charset_normalizer ({}) ' E + 'doesn\'t match a supported "', E ] ../datalad/tests/utils_pytest.py:107: AssertionError
This reverts commit 09cb41c. Original PR which removed upper bound: datalad#7029 Debians now have 5.1.0, and we can't package while demanding an outdated chardet. Let's see again if we really need to limit upper version.
#6777 is obsolete with psf/requests#6179. Could also remove the upper limit since it seems requests wants to drop the dependency eventually (psf/requests#6177 (comment)).
Changelog
🏠 Internal
requests
dependencies.