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.

Sign up for GitHub

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

Jump to bottom

Solr8 Preparatory Refactor #1 #4283

Merged
merged 7 commits into from
Feb 2, 2021
Merged

Solr8 Preparatory Refactor #1 #4283

merged 7 commits into from
Feb 2, 2021

Conversation

cdrini
Copy link
Collaborator

@cdrini cdrini commented Dec 15, 2020 •
edited
Loading

Work towards #3317 . Small cleanups of some solr-related code; from #2246 . Needed for solr8 update.

Note this will need a special deploy, with corresponding changes to our production openlibrary.yml in olsystem before we can test it.

Technical

  • Switch to base URL instead of host for Solr config -- needed since solr8 has a different base path (http://solr:8983/solr/openlibrary)
  • Switch to requests; solr8 was complaining about wrong contentType
  • "standard" isn't a valid solr wt, and on solr8 it ends up returning json instead of xml
  • Remove script/ol-solr-indexer.py ; unused in openlibrary or olsystem (container reference to old config format)

Testing

  • Locally, searching works; make reindex-solr works with solr8.
  • Searching works in staging.

Screenshot

Stakeholders

@tfmorris

@cdrini cdrini added the Needs: Special Deploy This PR will need a non-standard deploy to production label Dec 15, 2020
@cdrini cdrini changed the title Solr8 Preperatory Refactor #1 Solr8 Preparatory Refactor #1 Dec 15, 2020
@mekarpeles mekarpeles self-assigned this Dec 17, 2020
@cdrini cdrini mentioned this pull request Dec 28, 2020
Closed
31 tasks
@cdrini cdrini marked this pull request as draft December 28, 2020 18:27
@cdrini
Copy link
Collaborator Author

cdrini commented Dec 28, 2020

Converting to draft; need to test things still work with the urlparse change.

tfmorris and others added 7 commits December 29, 2020 12:50
@tfmorris @cdrini
Switch to base URL instead of host for Solr config
9c745df 
Co-authored-by: Drini Cami <cdrini@gmail.com>
@cdrini
Small tweaks to some solr code
98a70b5 
- Switch to requests; solr8 was complaining about wrong contentType
- "standard" isn't a valid solr wt, and on solr8 it ends up returning json instead of xml
- parsed_url.host should be hostname
@cdrini
Remove unused/deprecated update_work.get_solr
2bf5b55
@cdrini
Remove requests naming collision
10c89e1
@cdrini
Remove unused ol-solr-indexer.py
1104de9 
Confirmed unused here as well as in olsystem. Contained a reference to the old-style solr openlibrary.yml
@cdrini
Fix failing test_update_authors test
ce0ecfa
@cdrini @cclauss
Switch to six urlparse in update_work
5d6ecb4 
Co-authored-by: Christian Clauss <cclauss@me.com>
@cdrini cdrini force-pushed the refactor/solr8-prep branch from dcb3dd6 to 5d6ecb4 Compare December 29, 2020 17:51
@cdrini cdrini marked this pull request as ready for review December 29, 2020 18:12
@cdrini
Copy link
Collaborator Author

cdrini commented Dec 29, 2020

Tested, works. Putting on staging.

@cdrini cdrini added the On Staging On staging.openlibrary.org label Dec 29, 2020
@cdrini cdrini mentioned this pull request Dec 29, 2020
Merged
cclauss
cclauss approved these changes Jan 15, 2021
View reviewed changes
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. Linked to internetarchive/olsystem#108

@cdrini cdrini assigned cclauss and unassigned mekarpeles Feb 2, 2021
@cdrini cdrini merged commit 53e02a2 into internetarchive:master Feb 2, 2021
@cdrini cdrini deleted the refactor/solr8-prep branch February 2, 2021 21:55
@tfmorris tfmorris mentioned this pull request Feb 9, 2023
Closed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@cclauss cclauss cclauss approved these changes

Assignees

@cclauss cclauss

Labels
Needs: Special Deploy This PR will need a non-standard deploy to production On Staging On staging.openlibrary.org
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
4 participants
@cdrini @cclauss @mekarpeles @tfmorris