Class Option Request: Full requests output

[IanTBlack]

This is not an issue, but a change request.

Currently, when requests return a non-200 error, the ONC package returns an HTTPError.
For scripts that download large amounts of data, this may end up blocking subsequent requests if an error is raised and a user is not aware of all possibilities.

The following snippet...

params = {'locationCode': 'BACVP',
          'deviceCategoryCode': 'CTD',
          'dateFrom':'2025-01-01T00:00:00.000Z',
          'dateTo':'2025-01-01T23:59:59.999Z'}

data = onc.getScalardata(filters = params, allPages=True)

returns this error...

HTTPError: 
Status 400 - Bad Request: https://data.oceannetworks.ca/api/scalardata?locationCode=BACVP&deviceCategoryCode=CTD&dateFrom=2025-01-01T00%3A00%3A00.000Z&dateTo=2025-01-01T23%3A59%3A59.999Z&method=getByLocation&token=<>
API Error 127: A device with category CTD was deployed at location BACVP but not during the provided time range (20250101T000000.000Z to 20250101T235959.999Z). The deployment service can be used to determine a valid time range: https://data.oceannetworks.ca/api/deployments?locationCode=BACVP&deviceCategoryCode=CTD&token=<> (parameter: locationCode, deviceCategoryCode, dateFrom, dateTo)

The courteous thing to do would be to follow the recommendation of hitting the deployments endpoint, but like myself, users may just find it easier to force subsequent requests for data until data is returned. If not in a try-except statement that then parses the returned error, a script may stop.

Would it be possible to add a boolean keyword argument to the ONC class that instead offers the option of returning the full output from the request response, rather than raising an HTTPError? This would allow users to use the full functionality of the requests package and allow them to better tune download scripts for bad data requests or system downtimes.

For example...

import requests

onc = ONC(token = token, full_response_on_error = True)
response = onc.getScalardata(filters = params, allPages=True)

if response.status_code == requests.codes.ok:
     # Do stuff with the data.
elif response.status_code == requests.codes.bad:
    # Do stuff with the rest of the script.

[kan-fu]

Sure, it is a good idea to enable users to handle the request response object themselves as needed.

For the implementation, I plan to add a return before https://github.com/OceanNetworksCanada/api-python-client/blob/main/src/onc/modules/_OncService.py#L56 if a boolean flag (like returnRequestsObject) is True. Data Product Delivery and downloadArchivefile uses their own requests call to download the data instead of getting a json response, so those will not be affected by this boolean flag. Is that OK with you?

[IanTBlack]

I think that should work great!

[kan-fu]

There is an issue when dealing with the multi-page feature, which happens to be the case in your code snippet. if allPages==True, the client library repeatedly calls the web service if next in the response is not null, and concatenates multiple contents of requests.Response. So if we return the requests.Response at the first page, there is no way for the client library to grab the rest of the pages, and users need to manually query them. In this case it might be easier to use the try-except clause.

To summary, it looks like this full requests.Response output feature is not compatible with the multi-page feature. I think it defeats the purpose to implement it without supporting the multi-page feature. Any ideas on this issue?

See https://github.com/OceanNetworksCanada/api-python-client/blob/main/src/onc/modules/_OncRealTime.py#L97 and https://github.com/OceanNetworksCanada/api-python-client/blob/main/src/onc/modules/_MultiPage.py#L18

[IanTBlack]

Interesting. I hadn't considered compatibility with the multi-page feature. I agree, I don't think it is useful to implement without support for multi-page. I think originally I had thought it would only return responses for codes that aren't 2XX or 3XX. Are there situations where multiple pages are returned for bad requests or internal server issues?
Maybe a solution could be to return a list or tuple of responses?

Thanks for considering this, but if it is going to take significant rework of the package then maybe it should be sidelined.

[kan-fu]

I haven't worked on the backend service code, but I believe the next key in the response always has a valid url, which means if there are multiple pages (controlled by the rowLimit query parameter with a default value 80000), and the first page returns successfully, the whole multi-page requests will also succeed because it always follows the url in the next key.

For the possible solution to return a list of responses, I think it is impossible to know the pages without checking the response first. For example if we run https://data.oceannetworks.ca/api/scalardata?locationCode=BACVP&deviceCategoryCode=CTD&dateFrom=2009-08-20T00%3A00%3A00.000Z&dateTo=2009-08-29T23%3A59%3A59.999Z&method=getByLocation&token=TOKEN&rowLimit=10 (2009-08-20 ~ 2009-08-29), the next key in the response will point to https://data.oceannetworks.ca/api/scalardata?method=getByLocation&dateTo=2009-08-29T23%3A59%3A59.999Z&rowLimit=10&locationCode=BACVP&deviceCategoryCode=CTD&dateFrom=2009-08-27T00%3A53%3A32.289Z&token=TOKEN (2009-08-27 ~ 2009-08-29). The dateFrom and dateTo in the response are calculated in the backend service, and the client library cannot know or calculate them in advance just based on the url of the first page.

If we move on with the implementation without considering the multi-page feature, users can first call onc.getScalardata(filters = params) to check the response code (as the first page), then confidently call onc.getScalardata(filters = params, allPages=True) to grab all the pages if it returns successfully. It works, but I think it is not as straightforward as the try-excerpt clause. Yeah I agree it is better to leave it for now.

Let me know if you have any thoughts for improving the exception handling that might be useful for your experience.