From b50643814bc96f1878439e0bc0aebfcc4e5bdf15 Mon Sep 17 00:00:00 2001 From: Callum Sherry Date: Wed, 15 Jul 2026 14:39:53 -0700 Subject: [PATCH 1/5] Handle missing session metadata errors --- tests/test_utils.py | 66 +++++++++++++++++++++++++++++++++++++++++++++ utils.py | 10 ++++--- 2 files changed, 72 insertions(+), 4 deletions(-) create mode 100644 tests/test_utils.py diff --git a/tests/test_utils.py b/tests/test_utils.py new file mode 100644 index 00000000..aae762f8 --- /dev/null +++ b/tests/test_utils.py @@ -0,0 +1,66 @@ +import os +import sys +from unittest.mock import patch + +import pytest + + +thisDir = os.path.dirname(os.path.realpath(__file__)) +repoDir = os.path.abspath(os.path.join(thisDir,'../')) +sys.path.append(repoDir) +from utils import getCalibration, getNeutralTrialID + + +@patch('utils.getSessionJson') +def test_get_neutral_trial_id_from_session_trial(mock_get_session): + mock_get_session.return_value = { + 'trials': [ + {'id': 'dynamic-id', 'name': 'squats'}, + {'id': 'neutral-id', 'name': 'neutral'}, + ], + 'meta': {}, + } + + assert getNeutralTrialID('session-id') == 'neutral-id' + + +@patch('utils.getSessionJson') +def test_get_neutral_trial_id_from_metadata_fallback(mock_get_session): + mock_get_session.return_value = { + 'trials': [ + {'id': 'dynamic-id', 'name': 'squats'}, + ], + 'meta': { + 'neutral_trial': {'id': 'metadata-neutral-id'}, + }, + } + + assert getNeutralTrialID('session-id') == 'metadata-neutral-id' + + +@patch('utils.getSessionJson') +def test_get_neutral_trial_id_raises_clear_error_without_neutral_trial(mock_get_session): + mock_get_session.return_value = { + 'trials': [ + {'id': 'dynamic-id', 'name': 'squats'}, + ], + 'meta': {}, + } + + with pytest.raises(Exception, match='No neutral trial in session'): + getNeutralTrialID('session-id') + + +@patch('utils.getTrialJson') +@patch('utils.getCalibrationTrialID') +def test_get_calibration_raises_clear_error_without_camera_mapping( + mock_get_calibration_trial_id, mock_get_trial): + mock_get_calibration_trial_id.return_value = 'calibration-id' + mock_get_trial.return_value = { + 'results': [ + {'tag': 'calibration_parameters', 'media': 'calibration-url'}, + ], + } + + with pytest.raises(Exception, match='Redo calibration before processing dynamic trials'): + getCalibration('session-id', '/tmp/session') diff --git a/utils.py b/utils.py index 962aa74b..e7cca36b 100644 --- a/utils.py +++ b/utils.py @@ -855,10 +855,11 @@ def getNeutralTrialID(session_id): if len(neutral_ids)>0: neutralID = neutral_ids[-1] - elif session['meta']['neutral_trial']: - neutralID = session['meta']['neutral_trial']['id'] else: - raise Exception('No neutral trial in session.') + neutral_trial = (session.get('meta') or {}).get('neutral_trial') + if not neutral_trial: + raise Exception('No neutral trial in session.') + neutralID = neutral_trial['id'] return neutralID @@ -893,6 +894,8 @@ def getCalibration(session_id,session_path,trial_type='dynamic',getCalibrationOp # download the mapping videoFolder = os.path.join(session_path,'Videos') os.makedirs(videoFolder, exist_ok=True) + if 'camera_mapping' not in calibResultTags: + raise Exception('Calibration is missing camera mapping results. Redo calibration before processing dynamic trials.') mapURL = trial['results'][calibResultTags.index('camera_mapping')]['media'] mapLocalPath = os.path.join(videoFolder,'mappingCamDevice.pickle') download_file(mapURL,mapLocalPath) @@ -2121,4 +2124,3 @@ def makeRequestWithRetry(method, url, files=files) response.raise_for_status() return response - From 4bca787607d36a3f99a01f95335ecc9eac82cbba Mon Sep 17 00:00:00 2001 From: Callum Sherry Date: Thu, 16 Jul 2026 14:47:22 -0700 Subject: [PATCH 2/5] Use settings framerate for metadata updates --- tests/test_utils.py | 34 ++++++++++++++++++++++++++++++++++ utils.py | 7 ++----- 2 files changed, 36 insertions(+), 5 deletions(-) create mode 100644 tests/test_utils.py diff --git a/tests/test_utils.py b/tests/test_utils.py new file mode 100644 index 00000000..0b3fd63f --- /dev/null +++ b/tests/test_utils.py @@ -0,0 +1,34 @@ +import os +import sys +from unittest.mock import Mock, patch + + +thisDir = os.path.dirname(os.path.realpath(__file__)) +repoDir = os.path.abspath(os.path.join(thisDir,'../')) +sys.path.append(repoDir) +from utils import changeSessionMetadata + + +@patch('utils.getTrialJson') +@patch('utils.getNeutralTrialID') +@patch('utils.makeRequestWithRetry') +@patch('utils.getSessionJson') +def test_change_session_metadata_uses_settings_framerate_for_filterfrequency( + mock_get_session, mock_make_request, mock_get_neutral, mock_get_trial): + mock_get_session.return_value = { + 'meta': { + 'settings': { + 'framerate': 240, + }, + }, + } + mock_make_request.return_value = Mock(status_code=200) + mock_get_neutral.return_value = 'neutral-id' + mock_get_trial.return_value = { + 'results': [], + } + + changeSessionMetadata(['session-id'], {'filterfrequency': 100}) + + patched_meta = mock_make_request.call_args.kwargs['data']['meta'] + assert '"filterfrequency": "100"' in patched_meta diff --git a/utils.py b/utils.py index 962aa74b..a4757722 100644 --- a/utils.py +++ b/utils.py @@ -980,10 +980,8 @@ def changeSessionMetadata(session_ids,newMetaDict): existingMeta = session['meta'] # Check if framerate is in metadata. If not, set to 60 - if 'framerate' not in existingMeta: - framerate = 60 - else: - framerate = existingMeta['framerate'] + framerate = existingMeta.get('settings', {}).get( + 'framerate', existingMeta.get('framerate', 60)) if 'filterfrequency' in newMetaDict: if newMetaDict['filterfrequency'] != 'default': if float(newMetaDict['filterfrequency']) > framerate/2: @@ -2121,4 +2119,3 @@ def makeRequestWithRetry(method, url, files=files) response.raise_for_status() return response - From 1602082dc90b6b90d6edfff6d326ac52db296d99 Mon Sep 17 00:00:00 2001 From: Callum Sherry Date: Thu, 16 Jul 2026 16:43:45 -0700 Subject: [PATCH 3/5] Add request timeouts --- tests/test_api.py | 73 +++++++++++++++++++++++++++++++++-------------- utils.py | 14 ++++++--- 2 files changed, 61 insertions(+), 26 deletions(-) diff --git a/tests/test_api.py b/tests/test_api.py index 5a2c4e2b..96cc8508 100644 --- a/tests/test_api.py +++ b/tests/test_api.py @@ -2,33 +2,38 @@ import os import sys import pytest -from unittest.mock import patch, Mock -from http.client import HTTPMessage +from unittest.mock import patch, Mock, mock_open thisDir = os.path.dirname(os.path.realpath(__file__)) repoDir = os.path.abspath(os.path.join(thisDir,'../')) sys.path.append(repoDir) -from utils import makeRequestWithRetry +from utils import ( + DEFAULT_REQUEST_TIMEOUT, + UPLOAD_REQUEST_TIMEOUT, + makeRequestWithRetry, + uploadFileToS3, +) logging.getLogger('urllib3').setLevel(logging.DEBUG) @patch("requests.Session.request") -def test_get(mock_response): +def test_get(mock_request): status_code = 200 - mock_response.return_value.status_code = status_code + mock_request.return_value.status_code = status_code response = makeRequestWithRetry('GET', 'https://test.com', retries=2) assert response.status_code == status_code - mock_response.assert_called_once_with('GET', 'https://test.com', - headers=None, - data=None, - params=None, - files=None) + mock_request.assert_called_once_with('GET', 'https://test.com', + headers=None, + data=None, + params=None, + files=None, + timeout=DEFAULT_REQUEST_TIMEOUT) @patch("requests.Session.request") -def test_put(mock_response): +def test_put(mock_request): status_code = 201 - mock_response.return_value.status_code = status_code + mock_request.return_value.status_code = status_code data = { "key1": "value1", @@ -47,20 +52,44 @@ def test_put(mock_response): retries=2) assert response.status_code == status_code - mock_response.assert_called_once_with('POST', + mock_request.assert_called_once_with('POST', 'https://test.com', data=data, headers={"Authorization": "my_token"}, params=params, - files=None) + files=None, + timeout=DEFAULT_REQUEST_TIMEOUT) + +@patch("builtins.open", new_callable=mock_open, read_data=b"file-data") +@patch("utils.makeRequestWithRetry") +def test_upload_timeout(mock_request, mock_file): + mock_request.return_value.json.return_value = { + 'url': 'https://upload.test.com', + 'fields': {'key': 'uploaded-file'}, + } + + key = uploadFileToS3('/tmp/upload.mov') + + assert key == 'uploaded-file' + mock_request.assert_any_call('POST', + 'https://upload.test.com', + data={'key': 'uploaded-file'}, + files={'file': mock_file.return_value}, + timeout=UPLOAD_REQUEST_TIMEOUT) @patch("urllib3.connectionpool.HTTPConnectionPool._get_conn") -def test_success_after_retries(mock_response): - mock_response.return_value.getresponse.side_effect = [ - Mock(status=500, msg=HTTPMessage()), - Mock(status=502, msg=HTTPMessage()), - Mock(status=200, msg=HTTPMessage()), - Mock(status=429, msg=HTTPMessage()), +def test_success_after_retries(mock_get_conn): + def make_response(status): + response = Mock(status=status, headers={}) + response.stream.return_value = [] + response._original_response = None + return response + + mock_get_conn.return_value.getresponse.side_effect = [ + make_response(500), + make_response(502), + make_response(200), + make_response(429), ] response = makeRequestWithRetry('GET', @@ -69,11 +98,11 @@ def test_success_after_retries(mock_response): backoff_factor=0.1) assert response.status_code == 200 - assert mock_response.call_count == 3 + assert mock_get_conn.call_count == 3 # The httpbin test remains commented out for stability reasons # def test_httpbin(): # response = makeRequestWithRetry('GET', # 'https://httpbin.org/status/500', # retries=4, -# backoff_factor=0.1) \ No newline at end of file +# backoff_factor=0.1) diff --git a/utils.py b/utils.py index 962aa74b..7a414c81 100644 --- a/utils.py +++ b/utils.py @@ -28,6 +28,8 @@ API_URL = getAPIURL() API_TOKEN = getToken() +DEFAULT_REQUEST_TIMEOUT = (10, 10) +UPLOAD_REQUEST_TIMEOUT = (10, 300) DEPTH_DB_LEVEL = 10 DEPTH_DB_TRANSFORM = "vertical_delta_shuffle16" DEPTH_CONTAINER_MAGIC = b"OCDEPTHDB1\n" @@ -122,7 +124,8 @@ def uploadFileToS3(filePath): makeRequestWithRetry('POST', r['url'], data=r['fields'], - files=files) + files=files, + timeout=UPLOAD_REQUEST_TIMEOUT) return r['fields']['key'] @@ -2086,7 +2089,8 @@ def postProcessedDuration(trial_url, duration): # utils for common HTTP requests def makeRequestWithRetry(method, url, headers=None, data=None, params=None, files=None, - retries=5, backoff_factor=1): + retries=5, backoff_factor=1, + timeout=DEFAULT_REQUEST_TIMEOUT): """ Makes an HTTP request with retry logic and returns the Response object. @@ -2099,6 +2103,8 @@ def makeRequestWithRetry(method, url, params (dict): URL query parameters. retries (int): Number of retry attempts. backoff_factor (float): Backoff factor for exponential delays. + timeout (float or tuple): Seconds to wait for connection/response + activity, as accepted by requests.Session().request(). Returns: requests.Response: The response object for further processing. @@ -2118,7 +2124,7 @@ def makeRequestWithRetry(method, url, headers=headers, data=data, params=params, - files=files) + files=files, + timeout=timeout) response.raise_for_status() return response - From 6e22ddcddd9b5f23a4e9f7660daea9165539a1db Mon Sep 17 00:00:00 2001 From: Callum Sherry Date: Thu, 6 Aug 2026 15:39:35 -0700 Subject: [PATCH 4/5] Fix retry test response mock compatibility --- tests/test_api.py | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/tests/test_api.py b/tests/test_api.py index 96cc8508..11b6adea 100644 --- a/tests/test_api.py +++ b/tests/test_api.py @@ -3,6 +3,7 @@ import sys import pytest from unittest.mock import patch, Mock, mock_open +from http.client import HTTPMessage thisDir = os.path.dirname(os.path.realpath(__file__)) repoDir = os.path.abspath(os.path.join(thisDir,'../')) @@ -80,7 +81,7 @@ def test_upload_timeout(mock_request, mock_file): @patch("urllib3.connectionpool.HTTPConnectionPool._get_conn") def test_success_after_retries(mock_get_conn): def make_response(status): - response = Mock(status=status, headers={}) + response = Mock(status=status, msg=HTTPMessage(), headers={}) response.stream.return_value = [] response._original_response = None return response From 017d8fc0aa9ba870da20e4a53b68589b5528c363 Mon Sep 17 00:00:00 2001 From: Callum Sherry Date: Fri, 7 Aug 2026 13:39:39 -0700 Subject: [PATCH 5/5] Fix static time range retry logging --- main.py | 11 +++++++++-- 1 file changed, 9 insertions(+), 2 deletions(-) diff --git a/main.py b/main.py index 42e6af0f..2d16e8c8 100644 --- a/main.py +++ b/main.py @@ -545,6 +545,7 @@ def main(sessionName, trialName, trial_id, cameras_to_use=['all'], maxThreshold = 0.015 increment = 0.001 success = False + lastException = None while thresholdPosition <= maxThreshold and not success: try: timeRange4Scaling = getScaleTimeRange( @@ -553,8 +554,14 @@ def main(sessionName, trialName, trial_id, cameras_to_use=['all'], thresholdTime=0.1, removeRoot=True) success = True except Exception as e: - logging.info(f"Attempt identifying scaling time range with thresholdPosition {thresholdPosition} failed: {e}") - thresholdPosition += increment # Increase the threshold for the next iteration + lastException = e + logging.debug(f"Attempt identifying scaling time range with thresholdPosition {thresholdPosition} failed: {e}") + + if not success: + thresholdPosition += increment + + if not success: + raise lastException # Run scale tool. logging.info('Running Scaling')