Harden the scheduled HBNJ updater - #4
Merged
Conversation
Reviewer's GuideAdds configurable per-region retry handling with exponential backoff to the HBNJ daily updater, switches date handling to a fixed Japan Standard Time implementation, and hardens the Windows PowerShell wrapper to judge success by Python’s exit code while preserving existing validation and publishing flow. Sequence diagram for per-region retry with exponential backoff in HBNJ collectionsequenceDiagram
participant collect_hbnj_all_py as collect_hbnj_all.py_main
participant fetch
participant parse_region
collect_hbnj_all_py->>collect_hbnj_all_py: for each region
loop attempts 1..retries
collect_hbnj_all_py->>fetch: fetch(group_id, date)
alt fetch and parse succeed
fetch-->>collect_hbnj_all_py: source_url, source
collect_hbnj_all_py->>parse_region: parse_region(source, group_id, area_id, area_name, prefecture_raw, source_url)
parse_region-->>collect_hbnj_all_py: region
collect_hbnj_all_py->>collect_hbnj_all_py: break
else Exception during fetch or parse_region
collect_hbnj_all_py->>collect_hbnj_all_py: [attempt < retries] compute wait = retry_delay * 2^(attempt-1)
collect_hbnj_all_py->>collect_hbnj_all_py: time.sleep(wait)
end
end
collect_hbnj_all_py->>collect_hbnj_all_py: append area and channels
Sequence diagram for hardened Windows PowerShell wrapper around HBNJ daily updatersequenceDiagram
actor ScheduledTask
participant run_hbnj_daily_ps1 as run_hbnj_daily.ps1
participant update_hbnj_daily_py as update_hbnj_daily.py
ScheduledTask->>run_hbnj_daily_ps1: start script
run_hbnj_daily_ps1->>run_hbnj_daily_ps1: set PYTHONIOENCODING
run_hbnj_daily_ps1->>run_hbnj_daily_ps1: save previousErrorActionPreference
run_hbnj_daily_ps1->>run_hbnj_daily_ps1: set ErrorActionPreference = Continue
run_hbnj_daily_ps1->>update_hbnj_daily_py: & py -3 tools\update_hbnj_daily.py
update_hbnj_daily_py-->>run_hbnj_daily_ps1: LASTEXITCODE
run_hbnj_daily_ps1->>run_hbnj_daily_ps1: $updaterExitCode = $LASTEXITCODE
run_hbnj_daily_ps1->>run_hbnj_daily_ps1: restore ErrorActionPreference
alt updaterExitCode == 0
run_hbnj_daily_ps1-->>ScheduledTask: success
else updaterExitCode != 0
run_hbnj_daily_ps1->>run_hbnj_daily_ps1: throw "HBNJ updater exited with code $updaterExitCode"
run_hbnj_daily_ps1-->>ScheduledTask: failure
end
File-Level Changes
Tips and commandsInteracting with Sourcery
Customizing Your ExperienceAccess your dashboard to:
Getting Help
|
![sourcery-ai[bot]](https://avatars.githubusercontent.com/in/48477?s=60&v=4){kind=small}
There was a problem hiding this comment.
Hey - I've found 1 issue, and left some high level feedback:
- The per-region retry loop in
collect_hbnj_all.pycatchesException; consider narrowing this to network/HTTP/parsing-related exceptions so that truly unexpected errors (or interrupts) still fail fast. - You validate
--retriesand--retry-delayincollect_hbnj_all.pybut not inupdate_hbnj_daily.py; mirroring the basic validation there would prevent obviously invalid values from propagating into the downstream tool.
Prompt for AI Agents
Please address the comments from this code review:
## Overall Comments
- The per-region retry loop in `collect_hbnj_all.py` catches `Exception`; consider narrowing this to network/HTTP/parsing-related exceptions so that truly unexpected errors (or interrupts) still fail fast.
- You validate `--retries` and `--retry-delay` in `collect_hbnj_all.py` but not in `update_hbnj_daily.py`; mirroring the basic validation there would prevent obviously invalid values from propagating into the downstream tool.
## Individual Comments
### Comment 1
<location path="tools/run_hbnj_daily.ps1" line_range="14-19" />
<code_context>
+ # Windows PowerShell promotes native stderr to an ErrorRecord. This Python
+ # installation emits a harmless prefix warning on stderr, so temporarily
+ # allow the process to finish and judge success by its actual exit code.
+ $previousErrorActionPreference = $ErrorActionPreference
+ $ErrorActionPreference = "Continue"
& py -3 "tools\update_hbnj_daily.py" *>&1 | Tee-Object -FilePath $log
- if ($LASTEXITCODE -ne 0) {
- throw "HBNJ updater exited with code $LASTEXITCODE"
+ $updaterExitCode = $LASTEXITCODE
+ $ErrorActionPreference = $previousErrorActionPreference
+ if ($updaterExitCode -ne 0) {
+ throw "HBNJ updater exited with code $updaterExitCode"
}
</code_context>
<issue_to_address>
**suggestion:** ErrorActionPreference is restored only in the success path of capturing $updaterExitCode; consider guarding restoration with try/finally inside the inner block.
If an error occurs between changing `$ErrorActionPreference` and setting `$updaterExitCode` (e.g., a terminating error in the pipeline), the script will abort while still using the modified preference. Enclosing the `py` call and `$LASTEXITCODE` assignment in `try { ... } finally { $ErrorActionPreference = $previousErrorActionPreference }` ensures the original preference is always restored, even when that inner logic fails.
</issue_to_address>Help me be more useful! Please click 👍 or 👎 on each comment and I'll use the feedback to improve your reviews.
Comment on lines
+14
to
+19
| $previousErrorActionPreference = $ErrorActionPreference | ||
| $ErrorActionPreference = "Continue" | ||
| & py -3 "tools\update_hbnj_daily.py" *>&1 | Tee-Object -FilePath $log | ||
| if ($LASTEXITCODE -ne 0) { | ||
| throw "HBNJ updater exited with code $LASTEXITCODE" | ||
| $updaterExitCode = $LASTEXITCODE | ||
| $ErrorActionPreference = $previousErrorActionPreference | ||
| if ($updaterExitCode -ne 0) { |
There was a problem hiding this comment.
suggestion: ErrorActionPreference is restored only in the success path of capturing $updaterExitCode; consider guarding restoration with try/finally inside the inner block.
If an error occurs between changing $ErrorActionPreference and setting $updaterExitCode (e.g., a terminating error in the pipeline), the script will abort while still using the modified preference. Enclosing the py call and $LASTEXITCODE assignment in try { ... } finally { $ErrorActionPreference = $previousErrorActionPreference } ensures the original preference is always restored, even when that inner logic fails.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
What changed
Why
The 04:15 Scheduled Task failed before collection because Windows PowerShell promoted a harmless Python stderr warning to a terminating error. After that was exposed, the incomplete Python installation also lacked IANA timezone data. Both startup failures are removed and transient region failures now retry safely.
Checks
Summary by Sourcery
Harden the daily HBNJ collection and update workflow to be more resilient on Windows scheduled runs.
New Features:
Bug Fixes: