fix(hardware): allow details URLs without timestamps - #2117
Open
alanpeixinho wants to merge 2 commits into
Open
alanpeixinho wants to merge 2 commits into
alanpeixinho wants to merge 2 commits into
Conversation
Comment on lines
+43
to
+52
| def _lab_from_test(test: Tests) -> str | None: | ||
| ingester_lab = getattr(test, "_lab_name", None) | ||
| if ingester_lab: | ||
| return ingester_lab | ||
| lab_fk = getattr(test, "lab", None) | ||
| if lab_fk is not None: | ||
| return lab_fk.name | ||
| # TODO remove misc->>'runtime' fallback after lab backfill | ||
| misc = test.misc or {} | ||
| return misc.get("runtime") |
Member
There was a problem hiding this comment.
not sure this belongs here. making st/et optional doesn't need the lab fk fallback, or rewriting seed start_time to now.
the crash fix looks real, but it'd be easier to review/revert as its own PR (or at least mention it in the description)
Contributor
Author
There was a problem hiding this comment.
Thanks for pointing, this was left from local tests, and I will move to a separate PR.
Comment on lines
+14
to
+21
| const end = | ||
| endTimestampInSeconds ?? dateObjectToTimestampInSeconds(startOfTomorrow()); | ||
|
|
||
| return { | ||
| startTimestampInSeconds: | ||
| startTimestampInSeconds ?? end - daysToSeconds(intervalInDays), | ||
| endTimestampInSeconds: end, | ||
| }; |
Member
There was a problem hiding this comment.
this is a bit asymmetric. if only et is in the url, start is et - interval, but if only st is there, end becomes tomorrow instead of st + interval.
so a url that keeps an old st and drops et can turn into a huge window. maybe require both, or derive the missing one from whichever is present
Contributor
Author
There was a problem hiding this comment.
resolveHardwareTimeRange now treats a single bound as an interval-sized window.
- both st and et: use them as-is (existing bookmarks stay frozen)
- only et: st = et − intervalInDays
- only st: et = st + intervalInDays
- neither: rolling [tomorrow − interval, tomorrow)
alanpeixinho
marked this pull request as ready for review
September 18, 2026 13:55
* Make st/et optional on hardware details; default window is [tomorrow − iv, tomorrow] * Share that window between listing and details so counts stay aligned * Omit st/et from listing and test-detail links unless a revision is pinned * Link test hardware from platform/compatibles, not the Unknown label * Read lab from the test FK in aggregation backfill so ORM replays do not crash * Stamp seed start_time to now so local fixtures land in the default window Signed-off-by: Alan Peixinho <alan.peixinho@profusion.mobi>
* Derive the missing bound from whichever of st/et is present, so a URL with a stale st no longer widens the window to [st, tomorrow] * Drop the aggregation lab FK fallback and the seed start_time rewrite; neither is needed for optional st/et, and moving fixture start_time to now broke integration tests that pin historical timestamps Signed-off-by: Alan Peixinho <alan.peixinho@profusion.mobi>
alanpeixinho
force-pushed
the
fix/hardware-optional-timestamps
branch
from
September 23, 2026 12:58
1335b06 to
6a6e357
Compare
This branch has not been deployed
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
2 participants
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 it is
Hardware details no longer require
st/etin the URL. A bare/hardware/<id>uses a rolling window: end = start of tomorrow, start = end minusintervalInDays(URLi, default 5). Listing and details share the same resolver, so counts stay aligned when you changei(for example?i=100on listing) without freezing timestamps in the URL. Bookmarked URLs that already includest/etstill work.Closes #2110
How to test
/hardware/<id>with nost/etori. Page loads; API uses about 5 days ending at start of tomorrow./hardware?i=100, click a platform. Details URL hasi=100, nost/et; listing and details requests use the same about 100-day span.st/et. That window is preserved.