Skip to content

Commit

Permalink
Merge pull request #6 from NYPL/handle-e101
Browse files Browse the repository at this point in the history
Handle unrecognized site ID
  • Loading branch information
aaronfriedman6 authored Nov 14, 2024
2 parents 44c541c + 02235af commit 2f77f3d
Show file tree
Hide file tree
Showing 5 changed files with 40 additions and 10 deletions.
4 changes: 4 additions & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
@@ -1,3 +1,7 @@
## 2024-11-14 -- v1.0.4
### Fixed
- When site ID is not found (error code "E101"), skip it without throwing an error

## 2024-10-10 -- v1.0.3
### Fixed
- Retry when server is down (new ShopperTrak error code "E000")
Expand Down
7 changes: 4 additions & 3 deletions lib/pipeline_controller.py
Original file line number Diff line number Diff line change
Expand Up @@ -181,9 +181,10 @@ def _recover_data(self, site_dates, known_data_dict):
site_xml_root = self.shoppertrak_api_client.query(
SINGLE_SITE_ENDPOINT + row[0], row[1]
)
# If multiple sites match the same site ID (E104), continue to the next site
# ID. If the API limit has been reached (E107), stop.
if site_xml_root == "E104":
# If the site ID can't be found (E101) or multiple sites match the same site
# ID (E104), continue to the next site. If the API limit has been reached
# (E107), stop.
if site_xml_root == "E101" or site_xml_root == "E104":
continue
elif site_xml_root == "E107":
break
Expand Down
14 changes: 9 additions & 5 deletions lib/shoppertrak_api_client.py
Original file line number Diff line number Diff line change
Expand Up @@ -204,8 +204,16 @@ def _check_response(self, response_text):
) from None

if error is not None and error.text is not None:
# E000 is used when ShopperTrak is down and they recommend trying again
if error.text == "E000":
self.logger.info("E000: ShopperTrak is down")
return "E000"
# E101 is used when the given site ID is not recognized
elif error.text == "E101":
self.logger.warning("E101: site ID not found")
return "E101"
# E104 is used when the given site ID matches multiple sites
if error.text == "E104":
elif error.text == "E104":
self.logger.warning("E104: site ID has multiple matches")
return "E104"
# E107 is used when the daily API limit has been exceeded
Expand All @@ -216,10 +224,6 @@ def _check_response(self, response_text):
elif error.text == "E108":
self.logger.info("E108: ShopperTrak is busy")
return "E108"
# E000 is used when ShopperTrak is down and they recommend trying again
elif error.text == "E000":
self.logger.info("E000: ShopperTrak is down")
return "E000"
else:
self.logger.error(f"Error found in XML response: {response_text}")
raise ShopperTrakApiClientError(
Expand Down
5 changes: 3 additions & 2 deletions tests/test_pipeline_controller.py
Original file line number Diff line number Diff line change
Expand Up @@ -323,9 +323,9 @@ def test_recover_data(self, test_instance, mock_logger, mocker):
]
)

def test_recover_data_duplicate_sites(self, test_instance, mock_logger, mocker):
def test_recover_data_bad_sites(self, test_instance, mock_logger, mocker):
test_instance.shoppertrak_api_client.query.side_effect = [
_TEST_XML_ROOT, "E104", _TEST_XML_ROOT]
_TEST_XML_ROOT, "E104", "E101", _TEST_XML_ROOT]
mocked_process_recovered_data_method = mocker.patch(
"lib.pipeline_controller.PipelineController._process_recovered_data"
)
Expand All @@ -334,6 +334,7 @@ def test_recover_data_duplicate_sites(self, test_instance, mock_logger, mocker):
[
("aa", date(2023, 12, 1)),
("bb", date(2023, 12, 1)),
("cc", date(2023, 12, 1)),
("aa", date(2023, 12, 2)),
],
_TEST_KNOWN_DATA_DICT,
Expand Down
20 changes: 20 additions & 0 deletions tests/test_shoppertrak_api_client.py
Original file line number Diff line number Diff line change
Expand Up @@ -132,6 +132,19 @@ def test_query_request_exception(self, test_instance, requests_mock):
with pytest.raises(ShopperTrakApiClientError):
test_instance.query("test_endpoint", date(2023, 12, 31))

def test_query_unrecognized_site(self, test_instance, requests_mock, mocker):
requests_mock.get(
"https://test_shoppertrak_url/test_endpoint"
"?date=20231231&increment=15&total_property=N",
text="Error 101",
)
mocked_check_response_method = mocker.patch(
"lib.ShopperTrakApiClient._check_response", return_value="E101"
)

assert test_instance.query("test_endpoint", date(2023, 12, 31)) == "E101"
mocked_check_response_method.assert_called_once_with("Error 101")

def test_query_duplicate_sites(self, test_instance, requests_mock, mocker):
requests_mock.get(
"https://test_shoppertrak_url/test_endpoint"
Expand Down Expand Up @@ -207,6 +220,13 @@ def test_check_response(self, test_instance):
assert CHECKED_RESPONSE != "E108"
assert CHECKED_RESPONSE != "E000"

def test_check_response_unrecognized_site(self, test_instance):
assert test_instance._check_response(
'<?xml version="1.0" ?><message><error>E101</error><description>'
'The Customer Store ID supplied is not recognized by the system.'
'</description></message>'
) == "E101"

def test_check_response_duplicate_site(self, test_instance):
assert test_instance._check_response(
'<?xml version="1.0" ?><message><error>E104</error><description>The '
Expand Down

0 comments on commit 2f77f3d

Please sign in to comment.