Skip to content
New issue

Have a question about this project? # for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “#”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? # to your account

Refactor Solr/ES scripts + docs #1403

Merged
merged 6 commits into from
Nov 10, 2020
Merged

Refactor Solr/ES scripts + docs #1403

merged 6 commits into from
Nov 10, 2020

Conversation

lintool
Copy link
Member

@lintool lintool commented Nov 8, 2020

  • PEP8 compliance for scripts
  • Went through documentation, updated

@shaneding can you do a CR for me here also? Don't need to run the commands (I have), but at least sanity check to make sure I didn't screw up anything obvious.

@lintool lintool marked this pull request as draft November 8, 2020 12:14
@codecov
Copy link

codecov bot commented Nov 8, 2020

Codecov Report

Merging #1403 (c296c92) into master (a7996d1) will not change coverage.
The diff coverage is n/a.

Impacted file tree graph

@@            Coverage Diff            @@
##             master    #1403   +/-   ##
=========================================
  Coverage     54.72%   54.72%           
  Complexity      853      853           
=========================================
  Files           148      148           
  Lines          8591     8591           
  Branches       1217     1217           
=========================================
  Hits           4701     4701           
  Misses         3495     3495           
  Partials        395      395           

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update a7996d1...c296c92. Read the comment docs.

@lintool lintool requested a review from edwinzhng November 9, 2020 20:11
@lintool lintool marked this pull request as ready for review November 9, 2020 20:11
@lintool
Copy link
Member Author

lintool commented Nov 9, 2020

@adamyy can you take a quick look also, since you recently ran these experiments. Thanks!

Copy link
Member

@edwinzhng edwinzhng left a comment

Choose a reason for hiding this comment

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

LGTM, seems like everything is in order

try:
response = requests.get('http://localhost:8983/')
response.raise_for_status()
except: return False
else: return True
except requests.exceptions.RequestException:
Copy link
Member

Choose a reason for hiding this comment

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

There's no other exception that could be raised here? (and elsewhere)

@lintool lintool merged commit e19755b into master Nov 10, 2020
@lintool lintool deleted the es-solr branch November 10, 2020 12:47
# for free to join this conversation on GitHub. Already have an account? # to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants