-
-
Notifications
You must be signed in to change notification settings - Fork 4.3k
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
Upgrades Requests #6965
Upgrades Requests #6965
Conversation
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.
Hi @NishealJ, ptal!
appengine_config.py
Outdated
@@ -97,7 +97,7 @@ def save(self): | |||
os.path.join(ROOT_PATH, 'third_party', 'graphy-1.0.0'), | |||
os.path.join(ROOT_PATH, 'third_party', 'html5lib-python-1.0.1'), | |||
os.path.join(ROOT_PATH, 'third_party', 'mutagen-1.38'), | |||
os.path.join(ROOT_PATH, 'third_party', 'requests-2.10.0'), | |||
os.path.join(ROOT_PATH, 'third_party', 'requests-2.21.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.
The latest version of requests shown here is 2.22. We should be upgrading to 2.22 then.
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.
Hi @apb7, yes 2.22 is the latest but the changelog mentioned that they have officially stopped support for Python 3.4 as there was an ongoing project for migrating the code to python 3 I was not sure and i upgraded it to the latest feasible version
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.
No, I think we should be going for 2.22 since it supports Python 2.7 and >=3.5. At the moment, we're only making the code base easier to migrate to Python 3 (and not any specific version).
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.
Hi @apb7 Sounds good to me, I've updated the version and the changes are trivial. Thanks!
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.
Tests are failing with error
Parse error at third_party/generated/js/third_party.js:21644,12
import * as util from './util';
^
ERROR: Unexpected token: name «util», expected: punc «;»
I'll try to debug Thanks!
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.
Hi @apb7, The above failing tests has been resolved Thanks!
Codecov Report
@@ Coverage Diff @@
## develop #6965 +/- ##
========================================
Coverage 95.74% 95.74%
========================================
Files 371 371
Lines 51651 51651
========================================
Hits 49451 49451
Misses 2200 2200
Continue to review full report at Codecov.
|
Codecov Report
@@ Coverage Diff @@
## develop #6965 +/- ##
========================================
Coverage 95.73% 95.73%
========================================
Files 371 371
Lines 51604 51604
========================================
Hits 49404 49404
Misses 2200 2200
Continue to review full report at Codecov.
|
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.
LGTM, thanks, @NishealJ! Hi @nithusha21, @seanlip and @vojtechjelinek, can you please ptal as code owners? Thanks!
Explanation
This PR Upgrades Requests to its latest version,
to view the upgrade status of all the libraries please visit here
For testing doc please visit here
Checklist
python scripts/pre_commit_linter.py
andbash scripts/run_frontend_tests.sh
.