Add network fallback helper functions (#50 Task 2.4) by SBFRF · Pull Request #60 · SBFRF/murgtools

SBFRF · 2026-07-16T17:48:52Z

Summary

Adds open_dataset_with_fallback() for FRF -> CHL server fallback pattern
Adds open_dataset_with_retry() for retry loops with configurable attempts/delay
Refactors getWaveGaugeLoc() and getModelField() to use new helpers
Reduces code duplication and improves maintainability

Changes

Method	Before	After
`getWaveGaugeLoc()`	4-line try/except	1-line helper call
`getModelField()`	8-line while loop	2-line helper call

Test plan

All 173 existing tests pass
Added 10 new tests for helper functions
Tested primary URL success path
Tested fallback on IOError/OSError
Tested retry logic with max attempts
Tested config defaults

🤖 Generated with Claude Code

Consolidates duplicate try-except fallback patterns into reusable helpers: - open_dataset_with_fallback(): FRF -> CHL server fallback pattern - open_dataset_with_retry(): Retry loop with configurable attempts/delay Refactored: - getWaveGaugeLoc(): Now uses open_dataset_with_fallback() - getModelField(): Now uses open_dataset_with_retry() (was 15-iteration loop) Added 10 tests for helper functions covering: - Primary/fallback URL handling - IOError and OSError handling - Retry logic and config defaults - Error message printing Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>

Copilot

Pull request overview

This PR introduces reusable network-open helper functions in murgtools.getdata.getDataFRF to centralize the project’s common “fallback URL” and “retry open” patterns, and then refactors existing data-access methods to use them.

Changes:

Added open_dataset_with_fallback() (primary URL → fallback URL) and open_dataset_with_retry() (retry loop with delay/config default).
Refactored getWaveGaugeLoc() to use fallback helper and raise when both URLs fail.
Refactored getModelField() to use retry helper and raise a clearer RuntimeError when retries are exhausted; added tests for both helpers.

Reviewed changes

Copilot reviewed 2 out of 2 changed files in this pull request and generated 3 comments.

File	Description
`murgtools/getdata/getDataFRF.py`	Adds new dataset-open helpers and refactors callers (`getWaveGaugeLoc`, `getModelField`) to reduce duplicated network retry/fallback logic.
`tests/test_getDataFRF.py`	Adds unit tests covering success, fallback, retry exhaustion, and config-default behavior for the new helpers.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

+    Attempts to open a NetCDF dataset, retrying on IOError up to max_retries
+    times with a delay between attempts.


+        if ncfile is None:
+            raise IOError(f"Could not open dataset for gauge {gaugenumber}")


Refinements based on code review: - open_dataset_with_fallback(): Add raise_on_failure parameter for Pythonic exception handling. Default returns None, but callers can opt-in to exceptions. - open_dataset_with_retry(): Use logging module instead of print() for proper log levels (WARNING for retries, ERROR for final failure). Store last error for better error messages. - Update getWaveGaugeLoc() to use raise_on_failure=True for cleaner code - Update tests to use caplog fixture for logging assertions - Add test for raise_on_failure behavior - Add test for final failure logging Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>

Copilot

Pull request overview

Copilot reviewed 2 out of 2 changed files in this pull request and generated 3 comments.

+    Attempts to open a NetCDF dataset from the primary URL. If that fails
+    with an IOError (network issue, file not found), automatically tries
+    the fallback URL.


+    Returns:
+        netCDF4.Dataset: The opened dataset.


+    Attempts to open a NetCDF dataset, retrying on IOError up to max_retries
+    times with a delay between attempts. Logs progress using the logging module.


Addresses PR #60 review comments: - Rename parameter from max_retries to max_attempts to clarify that it represents total attempts (not additional retries after first try) - Add input validation requiring max_attempts >= 1 - Update docstring to explain semantics (max_attempts=3 means 3 total tries) - Update all callers and tests to use new parameter name Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>

Addresses PR #60 review comments: - Document that both IOError and OSError trigger fallback (not just IOError) - Fix return type to show it can return None when raise_on_failure=False Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>

SBFRF · 2026-07-16T18:28:41Z

@copilot please review the commits and check for clairty and capability

Copilot · 2026-07-16T18:31:05Z

@copilot please review the commits and check for clairty and capability

Reviewed the commits — all reviewer feedback has already been addressed in commits b22c321 and 449b7d8:

Docstring accuracy: Both open_dataset_with_fallback and open_dataset_with_retry now correctly document IOError or OSError (not just IOError)
Return type: open_dataset_with_fallback return type is documented as netCDF4.Dataset or None (not always a Dataset)
max_attempts clarity: Renamed from max_retries and documented as "Total number of attempts to make (not retries)" with an example clarifying the intent
Validation: open_dataset_with_retry raises ValueError when max_attempts < 1
Error messages: getWaveGaugeLoc() error now includes both attempted URLs via the helper: "Failed to open dataset from both {primary_url} and {fallback_url}"

All 157 tests pass including the 12 new helper-specific tests.

Copilot

Pull request overview

Copilot reviewed 2 out of 2 changed files in this pull request and generated 2 comments.

+    try:
+        return nc.Dataset(primary_url)
+    except (IOError, OSError):
+        pass  # Fall through to try fallback
+
+    try:
+        return nc.Dataset(fallback_url)
+    except (IOError, OSError) as e:
+        if raise_on_failure:
+            raise IOError(f"Failed to open dataset from both {primary_url} and {fallback_url}") from e
+        return None


+        except (IOError, OSError) as e:
+            last_error = e
+            if attempt < max_attempts - 1:
+                logging.warning(f"Error reading {url}, attempt {attempt + 1}/{max_attempts}. Retrying...")


Addresses PR #60 review comments: - open_dataset_with_fallback: Include both primary and fallback error messages when raise_on_failure=True, showing why each URL failed - open_dataset_with_retry: Include exception message in retry warning to help diagnose DNS, timeout, permission issues etc. Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>

Copilot AI review requested due to automatic review settings July 16, 2026 17:48

Copilot started reviewing on behalf of SBFRF July 16, 2026 17:49 View session

Copilot AI reviewed Jul 16, 2026

View reviewed changes

SBFRF requested a review from Copilot July 16, 2026 18:15

Copilot started reviewing on behalf of SBFRF July 16, 2026 18:16 View session

Copilot AI reviewed Jul 16, 2026

View reviewed changes

SBFRF and others added 2 commits July 16, 2026 14:21

Copilot started work on behalf of SBFRF July 16, 2026 18:29 View session

Copilot finished work on behalf of SBFRF July 16, 2026 18:31

SBFRF requested a review from Copilot July 16, 2026 18:34

Copilot started reviewing on behalf of SBFRF July 16, 2026 18:35 View session

Copilot AI reviewed Jul 16, 2026

View reviewed changes

SBFRF merged commit a2adf31 into main Jul 16, 2026
4 checks passed

SBFRF deleted the feature/issue-50-fallback-helper branch July 16, 2026 18:45

This was referenced Jul 16, 2026

[Phase 2] Performance Optimizations #50

Closed

create local data cache #9

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Add network fallback helper functions (#50 Task 2.4)#60

Add network fallback helper functions (#50 Task 2.4)#60
SBFRF merged 5 commits into
mainfrom
feature/issue-50-fallback-helper

SBFRF commented Jul 16, 2026

Uh oh!

Copilot AI left a comment

Uh oh!

Uh oh!

Copilot AI left a comment

Uh oh!

SBFRF commented Jul 16, 2026

Uh oh!

Copilot AI commented Jul 16, 2026

Uh oh!

Copilot AI left a comment

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

		Attempts to open a NetCDF dataset, retrying on IOError up to max_retries
		times with a delay between attempts.

		if ncfile is None:
		raise IOError(f"Could not open dataset for gauge {gaugenumber}")

		Attempts to open a NetCDF dataset, retrying on IOError up to max_retries
		times with a delay between attempts. Logs progress using the logging module.

Conversation

SBFRF commented Jul 16, 2026

Summary

Changes

Test plan

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Reviewed changes

Uh oh!

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Uh oh!

SBFRF commented Jul 16, 2026

Uh oh!

Copilot AI commented Jul 16, 2026

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants