feat(amber): expose the warehouse owner in DashboardWarehouse - #7745
Open
mengw15 wants to merge 1 commit into
Open
feat(amber): expose the warehouse owner in DashboardWarehouse#7745mengw15 wants to merge 1 commit into
mengw15 wants to merge 1 commit into
Conversation
GET /warehouse/status returned DashboardWarehouse without owner information, so the warehouse dashboard tab and picker could only render the owner avatar from the currently signed-in user -- correct only while warehouses are strictly per-user, and wrong as soon as they can be shared. Mirror how computing units model this: add ownerName / ownerAvatar to DashboardWarehouse, resolved per entry from the user table (null when the user has no name or avatar set), batched over the distinct owner uids of a listing. The UI can then bind each entry to its own owner instead of the session user. WarehouseResourceSpec asserts the fields on both mapping paths: create returns the caller's name with a null avatar for the avatar-less fixture user, and status resolves another user's name and avatar per entry. Part of apache#6870, follow-up to apache#6932. Closes apache#7743.
Contributor
Automated Reviewer SuggestionsBased on the
|
Contributor
There was a problem hiding this comment.
Pull request overview
Adds per-warehouse owner metadata to dashboard API responses.
Changes:
- Batches owner name/avatar resolution from the user table.
- Populates owner fields for warehouse listing and creation responses.
- Tests populated and null owner metadata.
Reviewed changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated no comments.
| File | Description |
|---|---|
WarehouseResource.scala |
Adds and resolves owner fields. |
WarehouseResourceSpec.scala |
Verifies owner metadata mappings. |
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
Contributor
|
| config | throughput | MB/s | latency | max Δ latest / 7d | |
|---|---|---|---|---|---|
| 🔴 | bs=10 sw=10 sl=64 | 534 | 0.326 | 18,296/23,675/23,675 us | 🟢 -10.5% / 🔴 +48.1% |
| 🔴 | bs=100 sw=10 sl=64 | 1,220 | 0.745 | 81,308/102,079/102,079 us | 🔴 +15.4% / 🟢 +20.7% |
| ⚪ | bs=1000 sw=10 sl=64 | 1,408 | 0.86 | 708,211/795,019/795,019 us | ⚪ within ±5% / 🟢 +34.3% |
Baseline details
Latest main 60300e3 from same runner
| config | metric | PR | latest main | 7d avg | Δ latest | Δ 7d |
|---|---|---|---|---|---|---|
| bs=10 sw=10 sl=64 | throughput | 534 tuples/sec | 516 tuples/sec | 779.07 tuples/sec | +3.5% | -31.5% |
| bs=10 sw=10 sl=64 | MB/s | 0.326 MB/s | 0.315 MB/s | 0.476 MB/s | +3.5% | -31.4% |
| bs=10 sw=10 sl=64 | p50 | 18,296 us | 17,346 us | 12,818 us | +5.5% | +42.7% |
| bs=10 sw=10 sl=64 | p95 | 23,675 us | 26,465 us | 15,986 us | -10.5% | +48.1% |
| bs=10 sw=10 sl=64 | p99 | 23,675 us | 26,465 us | 19,339 us | -10.5% | +22.4% |
| bs=100 sw=10 sl=64 | throughput | 1,220 tuples/sec | 1,262 tuples/sec | 1,011 tuples/sec | -3.3% | +20.6% |
| bs=100 sw=10 sl=64 | MB/s | 0.745 MB/s | 0.77 MB/s | 0.617 MB/s | -3.2% | +20.7% |
| bs=100 sw=10 sl=64 | p50 | 81,308 us | 79,534 us | 100,965 us | +2.2% | -19.5% |
| bs=100 sw=10 sl=64 | p95 | 102,079 us | 88,468 us | 107,295 us | +15.4% | -4.9% |
| bs=100 sw=10 sl=64 | p99 | 102,079 us | 88,468 us | 115,531 us | +15.4% | -11.6% |
| bs=1000 sw=10 sl=64 | throughput | 1,408 tuples/sec | 1,404 tuples/sec | 1,049 tuples/sec | +0.3% | +34.3% |
| bs=1000 sw=10 sl=64 | MB/s | 0.86 MB/s | 0.857 MB/s | 0.64 MB/s | +0.4% | +34.3% |
| bs=1000 sw=10 sl=64 | p50 | 708,211 us | 715,305 us | 978,248 us | -1.0% | -27.6% |
| bs=1000 sw=10 sl=64 | p95 | 795,019 us | 797,301 us | 1,021,881 us | -0.3% | -22.2% |
| bs=1000 sw=10 sl=64 | p99 | 795,019 us | 797,301 us | 1,050,075 us | -0.3% | -24.3% |
Raw CSV
config_idx,batch_size,schema_width,string_len,num_batches,total_ms,total_tuples,total_bytes,tuples_per_sec,mb_per_sec,lat_p50_us,lat_p95_us,lat_p99_us
0,10,10,64,20,374.45,200,128000,534,0.326,18295.94,23674.79,23674.79
1,100,10,64,20,1639.42,2000,1280000,1220,0.745,81307.90,102079.47,102079.47
2,1000,10,64,20,14201.99,20000,12800000,1408,0.860,708210.60,795019.22,795019.22
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## main #7745 +/- ##
=========================================
Coverage 91.02% 91.03%
- Complexity 4454 4455 +1
=========================================
Files 1174 1174
Lines 47146 47159 +13
Branches 5287 5288 +1
=========================================
+ Hits 42916 42930 +14
Misses 2550 2550
+ Partials 1680 1679 -1
*This pull request uses carry forward flags. Click here to find out more. ☔ View full report in Codecov by Harness. 🚀 New features to boost your workflow:
|
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.
What changes were proposed in this PR?
GET /warehouse/statusreturnsDashboardWarehouse(whid, name, warehouseName, flavor, createdAtMillis)with no owner information, so the warehouse dashboard tab and picker (#6933) can only render the owner avatar from the currently signed-in user. That is correct only while warehouses are strictly per-user, and would show the wrong person as soon as warehouses can be shared. Computing units already model this properly:DashboardWorkflowComputingUnitcarriesownerName/ownerAvatarresolved per entry.ownerNameandownerAvatartoDashboardWarehouse, mirroring the computing-unit semantics: resolved from the user table per entry, null when the user has no name or avatar set.status()listing and thecreate()response.Any related issues, documentation, discussions?
Closes #7743. Part of #6870, follow-up to #6932; mirrors
DashboardWorkflowComputingUnit's owner fields.How was this PR tested?
WarehouseResourceSpec(embedded Postgres + stubbedLakekeeperClient, no external infra) asserts the fields on both mapping paths:createreturns the caller'sownerNamewith a nullownerAvatarfor the avatar-less fixture user, andstatusresolves another user's name and avatar per entry.WorkflowExecutionService/scalafmtCheck(main + Test) passes.Was this PR authored or co-authored using generative AI tooling?
Generated-by: Claude Code (claude-opus-4-8)