Skip to content

Conversation

@tpoliaw
Copy link
Contributor

@tpoliaw tpoliaw commented Dec 3, 2025

On the instrumentSession object, run takes a scan number as these should
be unique within a session. On the root, it takes a run id as these
should be unique across tiled.

@tpoliaw tpoliaw linked an issue Dec 3, 2025 that may be closed by this pull request
@codecov
Copy link

codecov bot commented Dec 3, 2025

Codecov Report

❌ Patch coverage is 0% with 21 lines in your changes missing coverage. Please review.
✅ Project coverage is 49.27%. Comparing base (6fb304d) to head (1ca9994).
⚠️ Report is 1 commits behind head on main.

Files with missing lines Patch % Lines
src/model.rs 0.00% 10 Missing ⚠️
src/clients.rs 0.00% 8 Missing ⚠️
src/model/node.rs 0.00% 3 Missing ⚠️
Additional details and impacted files
@@            Coverage Diff             @@
##             main      #89      +/-   ##
==========================================
- Coverage   51.00%   49.27%   -1.73%     
==========================================
  Files          13       13              
  Lines         600      621      +21     
==========================================
  Hits          306      306              
- Misses        294      315      +21     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

Copy link
Contributor

@abbiemery abbiemery left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I like being able to look at one run, but now if you use the instrument session query you have both run and runs as a field option. Is this what you expected? While it became quickly apparent what is was for after graphiql shouted at me, it felt a bit confusing on the face of it. I don't have an opinion yet on how i'd want this to feel, but I thought i'd mention it.
image

Also, in my data I actually have two scans with different run_id's but the same scan number. This change only grabs one of them, not sure how it decides which. While in the world of production we keep track of these numbers to avoid clashes, anyone messing about in the weeds with our stack (Im thinking training rigs) may have this happen too. I think the choices are: return both things, Or specify on run _id instead. It's nicer for the user to be able to avoid uuid's but then we have a ui that isn't consistent.

image

@tpoliaw
Copy link
Contributor Author

tpoliaw commented Dec 8, 2025

you have both run and runs as a field option. Is this what you expected?

Yes, one gets a single run, the other gets all the runs. Open to better naming ideas. all_runs and run?

This change only grabs one of them, not sure how it decides which

The first one returned by tiled. The problem with returning all matching scans is that you either have to always return a list when in the vast majority of cases with real data you're only going to get a single result, or have a flexible schema where you have to handle both cases which is a pain.

Or specify on run _id instead.

If the user has the run_id they don't need to go via the instrument_session, they can access the run directly via the run endpoint.

@tpoliaw tpoliaw force-pushed the single-run-queries branch 2 times, most recently from 3b02fc0 to f759adb Compare December 8, 2025 16:39
On the instrumentSession object, run takes a scan number as these should
be unique within a session. On the root, it takes a run id as these
should be unique across tiled.
@tpoliaw tpoliaw force-pushed the single-run-queries branch from f759adb to 04a90ed Compare December 8, 2025 17:20
@tpoliaw tpoliaw force-pushed the single-run-queries branch from 9819837 to 96e64d7 Compare December 9, 2025 11:49
Copy link
Contributor

@abbiemery abbiemery left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I do think this is better like this with instrument session only having runs. Going forward we should enable runs to take arguments to filter on.

@tpoliaw tpoliaw merged commit be5396d into main Dec 9, 2025
5 of 7 checks passed
@tpoliaw tpoliaw deleted the single-run-queries branch December 9, 2025 13:26
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Add a query for a run on a given instrument session

3 participants