Repository navigation
Conversation
The GOES import runs hourly and slices exactly one hour back from Time.now(). NOAA publishes each minute a few minutes late, so the newest point available when the job fires at :54:56 is around :49, and the next run's window starts at :54:56, after the minutes that arrived in the meantime. Nothing ever fetches them again. Timestream shows the hole: over 24 hours every minute of the hour has 23 or 24 samples except :50 with 13, :51 with 6, and :52 through :54 with none at all. That is roughly two hours of X-ray flux a day that never lands. Look back two hours instead. The feed already returns six hours on every call, so this costs no extra request. Timestream deduplicates identical records and record_timeseries logs a rejected duplicate rather than raising, so the overlap is free, and a run that fails now backfills itself on the next hour.
There was a problem hiding this comment.
🟢 Approval recommended
The changes address delayed measurements with regression coverage and no unresolved blocking issues.
Pull request overview
Extends the GOES import lookback to two hours to capture delayed NOAA measurements.
Changes:
- Adds an overlapping two-hour import window.
- Adds regression coverage for delayed points.
File summaries
| File | Summary |
|---|---|
lambda_function/tests/test_executor.py |
Tests recording stale and recent measurements. |
lambda_function/src/executor/executor.py |
Implements the two-hour GOES import window. |
Review details
- Files reviewed: 2/2 changed files
- Comments generated: 0
- Review effort level: Lite
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
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
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.
The gap
import_GOES_data_to_timestreamruns hourly and slices exactly one hour back fromTime.now():NOAA publishes each minute a few minutes late. The rule fires at
:54:56, so the newest point in the feed at that moment is around:49. The minutes that arrive after the run starts are never picked up, because the next run's window begins at:54:56— already past them.Timestream shows the hole. Sample counts per minute-of-hour over 24 hours:
:00–:49:50:51:52–:54:55–:59That is roughly two hours of GOES X-ray flux per day that never lands, every day.
The change
Look back two hours instead of one, so every minute falls inside two consecutive runs.
This costs nothing extra:
xrays-6-hour.json, so six hours already come back on every call — no additional requestrecord_timeseriescatchesRejectedRecordsExceptionand logs it rather than raising (swxsoc/db/timeseries.py:377), so a revised value that loses a version check cannot fail the jobIt also makes the job self-healing: a run that fails now gets backfilled by the next hour instead of leaving a permanent hole.
Test
Added
test_import_GOES_data_covers_the_minutes_between_runs, which stubs the feed with one point 70 minutes old and one 5 minutes old and asserts both are recorded. It fails on the old one-hour window (records 1 of 2) and passes on the new one. No network access.Verified locally: the new test fails before the change and passes after. Full suite is 8 passed, with
test_import_stix_to_timestreamandtest_get_padre_orbit_datafailing only becausepadre_craftandstixdcpywere not installed in the local environment — neither touches this code path and CI installs the full requirements.Not included
Separately,
goes_fluxcurrently plots as a gap on dashboards roughly 08:30–09:30 UTC daily. That is not a pipeline fault: the stored values are exactly0.0during satellite eclipse, and a log-scale axis cannot render zero. The window has been lengthening toward the September equinox (51 zero-minutes on 09-11, 59 on 09-16) and will taper off through October. Worth handling as a null in the panel query rather than here.