Skip to content
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

refactor update_work and test_update_work to use requests #3696

Merged

Conversation

aasifkhan7
Copy link
Contributor

Closes #2852
related to #3694

@cclauss Refactor only update_work.py and test_update_work.py

@cclauss
Copy link
Contributor

cclauss commented Aug 14, 2020

Copy link
Contributor

@cclauss cclauss left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

@aasifkhan7
Copy link
Contributor Author

I also see this in the changed files - @cclauss - not very sure about this, but it shouldn't be present, right? maybe i did git add . at some point which committed the submodule changes too.

Screenshot 2020-08-15 at 9 04 36 AM

@aasifkhan7
Copy link
Contributor Author

aasifkhan7 commented Aug 15, 2020

@cclauss I just confirmed - this does change the submodule settings for infogami - it checks it out to some other commit, which I had done locally. will try to revert that. 😅

@aasifkhan7 aasifkhan7 force-pushed the modify-update_work-to-requests branch from 13570f3 to 1db9d23 Compare August 15, 2020 03:50
@aasifkhan7
Copy link
Contributor Author

Done @cclauss

@cclauss
Copy link
Contributor

cclauss commented Aug 15, 2020

@cdrini @mekarpeles @hornc I have approved this PR. It would be good to get your reviews.

Copy link
Collaborator

@cdrini cdrini left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Lgtm! I tested make reindex-solr works and logs urlopen without errors

openlibrary/solr/update_work.py Outdated Show resolved Hide resolved
@cdrini cdrini merged commit eccf887 into internetarchive:master Sep 8, 2020
aasifkhan7 added a commit to aasifkhan7/openlibrary that referenced this pull request Sep 8, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Replace all urllib / urllib2 imports and uses with requests
3 participants