Skip to content

Commit

Permalink
Don't log ping 404 as an error response
Browse files Browse the repository at this point in the history
Adds optional allowed reponse parameter to solr base client
make_request method
  • Loading branch information
rlskoeser committed Apr 4, 2019
1 parent c652a2d commit b102eab
Show file tree
Hide file tree
Showing 3 changed files with 29 additions and 9 deletions.
9 changes: 5 additions & 4 deletions parasolr/solr/admin.py
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,7 @@
from attrdict import AttrDict
import requests

from parasolr.solr.client import ClientBase
from parasolr.solr.base import ClientBase


class CoreAdmin(ClientBase):
Expand Down Expand Up @@ -79,8 +79,9 @@ def ping(self, core: str) -> bool:
"""
ping_url = '/'.join([self.solr_url.rstrip('/'), core, 'admin', 'ping'])
response = self.make_request('get', ping_url)
# ping returns 404 if core does not exist (make request returns None)

# ping returns 404 if core does not exist, but that's ok here
allowed_responses = [requests.codes.ok, requests.codes.not_found]
response = self.make_request('get', ping_url,
allowed_responses=allowed_responses)
# return True if response is valid and status is OK
return response and response.status == 'OK'
23 changes: 20 additions & 3 deletions parasolr/solr/base.py
Original file line number Diff line number Diff line change
Expand Up @@ -62,21 +62,28 @@ def build_url(self, solr_url: str, collection: str, handler: str) -> str:

def make_request(self, meth: str, url: str, headers: Optional[dict]=None,
params: Optional[dict]=None, data: Optional[dict]=None,
wrap: bool=True, **kwargs: Any) -> Optional[AttrDict]:
"""Make an HTTP request to Solr.
wrap: bool=True, allowed_responses: Optional[list]=None,
**kwargs: Any) -> Optional[AttrDict]:
"""Make an HTTP request to Solr. May optionally specify a list of
allowed HTTP status codes for this request. Responses will be
logged as errors if they are not in the list, but only responses
with 200 OK status will be loaded as JSON.
Args:
meth: HTTP method to use.
url: URL to make request to.
headers: HTTP headers.
params: Params to use as form-fields or query-string params.
data: Data for a POST request.
allowed_responses: HTTP status codes that are allowed for this
request; if not set, defaults to 200 OK
**kwargs: Any other kwargs for the request.
"""

if params is None:
params = dict()
# always add wt=json for JSON api

# copy user-supplied params for inclusion in debug logging
user_params = params.copy()
params['wt'] = 'json'
Expand Down Expand Up @@ -104,7 +111,11 @@ def make_request(self, meth: str, url: str, headers: Optional[dict]=None,
time.time() - start,
'\n%s' % user_params if user_params else '',
)
if response.status_code != requests.codes.ok:

if allowed_responses is None:
allowed_responses = [requests.codes.ok]

if response.status_code not in allowed_responses:
# Add the content of the response on the off chance
# it's helpful
logger.error(
Expand All @@ -118,6 +129,12 @@ def make_request(self, meth: str, url: str, headers: Optional[dict]=None,

# do further error checking on the response because Solr
# may return 200 but pass along its own error codes and information

# if response was allowed but not a 200, just
# return response instead of attempting to load as json
if response.status_code != requests.codes.ok:
return response

output = AttrDict(response.json())
if 'responseHeader' in output \
and output.responseHeader.status != 0:
Expand Down
6 changes: 4 additions & 2 deletions parasolr/solr/test_solr.py
Original file line number Diff line number Diff line change
Expand Up @@ -101,7 +101,7 @@ def core_test_client(request):
are always cleaned up on teardown.
"""
solr_url = TEST_SOLR_CONNECTION.get('URL', None)
commitWithin = TEST_SOLR_CONNECTION.get('COMMITWITHIN', None)
commitWithin = TEST_SOLR_CONNECTION.get('COMMITWITHIN', None)

if not solr_url:
raise ImproperConfiguration(
Expand Down Expand Up @@ -646,10 +646,12 @@ def test_reload(self, test_client):
assert test_client.core_admin.reload(test_client.collection)
assert not test_client.core_admin.reload('foo')

def test_ping(self, core_test_client):
def test_ping(self, core_test_client, caplog):
# ping should return false for non-existent core
solrclient, core = core_test_client
assert not solrclient.core_admin.ping(core)
# should not log the error since 404 is an allowed response for ping
assert not caplog.records
# create the core and then check it
solrclient.core_admin.create(core, configSet='basic_configs',
dataDir='foo')
Expand Down

0 comments on commit b102eab

Please sign in to comment.