-
Notifications
You must be signed in to change notification settings - Fork 111
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
Revert "Remove chardet version upper limit" to bring back upper limit <= 5.0.0 on chardet #7057
Conversation
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
Codecov ReportBase: 75.05% // Head: 76.17% // Increases project coverage by
Additional details and impacted files@@ Coverage Diff @@
## maint #7057 +/- ##
==========================================
+ Coverage 75.05% 76.17% +1.12%
==========================================
Files 354 354
Lines 58927 58940 +13
Branches 6615 6615
==========================================
+ Hits 44226 44896 +670
+ Misses 14687 14030 -657
Partials 14 14
Help us with your feedback. Take ten seconds to tell us how you rate us. Have a feature suggestion? Share it here. ☔ View full report at Codecov. |
0e15c9d
to
52ab90f
Compare
if no objections, I would merge this asap (ie tomorrow) to bring CI back to sane state. |
As promised -- merging |
For some reason, pip install requests 2.27.1 in Travis CI while the chardet version bump is in 2.28.1. Until someone can fix the discrepancy or version in Travis CI catches up, I agree we should put #7029 on hold. |
PR released in |
This reverts commit 0f27cd5.
Original change sounded kosher and "we agreed" with reasoning but original PR #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
@kimsin98 feel welcome to reincarnate #7029 but we should make sure it doesn't break testing again -- there must be more to the story.