Skip to content

Commit

Permalink
Handle Quickbase API errors
Browse files Browse the repository at this point in the history
Why these changes are being introduced:

Quickbase API may surface errors in two ways.  First, there
might be a non 2xx status code returned, which we will throw
and immediate exception.  Second, the request may have been
partially successful, but the response indicates some failures
for certain data upserted.  For these we will log but continue.

How this addresses that need:
* Adds status code error handling in QBClient
* Adds logging to QB tasks run method when errors in response

Side effects of this change:
* None

Relevant ticket(s):
* https://mitlibraries.atlassian.net/browse/HRQB-17
  • Loading branch information
ghukill committed May 16, 2024
1 parent 6ae6b96 commit 04d1796
Show file tree
Hide file tree
Showing 4 changed files with 62 additions and 4 deletions.
9 changes: 9 additions & 0 deletions hrqb/base/task.py
Original file line number Diff line number Diff line change
Expand Up @@ -196,6 +196,10 @@ def run(self) -> None:
Because Load Tasks (upserting data to Quickbase) are so uniform, this run method
can be defined on this base class. All data required for this operation exists
on the Task: data from parent Transform class and QB table name.
Partial successes are possible for Quickbase upserts. This method will log
warnings when detected in API response, but task will be considered complete and
ultimately successful.
"""
records = self.get_records()

Expand All @@ -208,6 +212,11 @@ def run(self) -> None:
)
results = qbclient.upsert_records(upsert_payload)

# log warning, but consider task complete if some errors present in API response
if api_errors := results.get("metadata", {}).get("lineErrors"):
message = f"Quickbase API call completed but had errors: {api_errors}"
logger.warning(message)

self.target.write(results)


Expand Down
19 changes: 15 additions & 4 deletions hrqb/utils/quickbase.py
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,7 @@
import pandas as pd
import requests
from attrs import define, field
from requests.exceptions import RequestException

from hrqb.config import Config
from hrqb.exceptions import QBFieldNotFoundError
Expand Down Expand Up @@ -50,15 +51,25 @@ def make_request(
return self._cache[request_hash]

# make API call
results = requests_method(
response = requests_method(
f"{self.api_base}/{path.removeprefix('/')}",
headers=self.request_headers,
**kwargs,
).json()
)

# handle non 2xx responses
if not 200 <= response.status_code < 300: # noqa: PLR2004
message = (
f"Quickbase API error - status {response.status_code}, "
f"content: {response.text}"
)
raise RequestException(message)

data = response.json()
if self.cache_results:
self._cache[request_hash] = results
self._cache[request_hash] = data

return results
return data

def get_app_info(self) -> dict:
"""Retrieve information about the QB app.
Expand Down
25 changes: 25 additions & 0 deletions tests/test_base_task.py
Original file line number Diff line number Diff line change
Expand Up @@ -147,6 +147,31 @@ def test_quickbase_task_run_upsert_and_json_receipt_output_target_success(
assert task_load_animals.target.read() == mocked_qb_upsert_receipt


def test_quickbase_task_run_upsert_and_json_receipt_output_target_api_errors_logged(
caplog, task_transform_animals_target, task_load_animals
):
"""Mocks upsert to Quickbase, asserting mocked response is written as Target data"""
mocked_qb_upsert_receipt = {
"data": [],
"metadata": {
"createdRecordIds": [11, 12],
"lineErrors": {"2": ['Incompatible value for field with ID "6".']},
"totalNumberOfRecordsProcessed": 3,
"unchangedRecordIds": [],
"updatedRecordIds": [],
},
}
with mock.patch("hrqb.base.task.QBClient", autospec=True) as mock_qbclient_class:
mock_qbclient = mock_qbclient_class()
mock_qbclient.get_table_id.return_value = "abcdef123"
mock_qbclient.prepare_upsert_payload.return_value = {}
mock_qbclient.upsert_records.return_value = mocked_qb_upsert_receipt

task_load_animals.run()

assert "Quickbase API call completed but had errors" in caplog.text


def test_base_pipeline_name(task_pipeline_animals):
assert task_pipeline_animals.pipeline_name == "Animals"

Expand Down
13 changes: 13 additions & 0 deletions tests/test_qbclient_client.py
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,8 @@

import pandas as pd
import pytest
import requests
from requests.models import Response

from hrqb.exceptions import QBFieldNotFoundError

Expand Down Expand Up @@ -162,3 +164,14 @@ def test_qbclient_get_table_as_df_some_fields(qbclient_with_mocked_table_fields)
)
assert list(records_df.columns) == fields
assert "Date time" not in records_df.columns


def test_qbclient_api_non_2xx_response_error(qbclient):
with mock.patch.object(requests, "get") as mocked_get:
response = Response()
response.status_code = 400
mocked_get.return_value = response
with pytest.raises(
requests.RequestException, match="Quickbase API error - status 400"
):
assert qbclient.make_request(requests.get, "/always/fail")

0 comments on commit 04d1796

Please sign in to comment.