Skip to content

feat(amber): reject invalid dashboard resource type - #8162

Closed
carloea2 wants to merge 1 commit into
apache:mainfrom
carloea2:fix/dashboard-resource-type-response
Closed

feat(amber): reject invalid dashboard resource type#8162
carloea2 wants to merge 1 commit into
apache:mainfrom
carloea2:fix/dashboard-resource-type-response

Conversation

@carloea2

Copy link
Copy Markdown
Contributor

What changes were proposed in this PR?

Return HTTP 400 when dashboard search receives an unknown resource type.

The resource dispatch currently throws IllegalArgumentException, which Dropwizard reports as HTTP 500. The change uses BadRequestException and updates both invalid input cases in the focused spec.

Before: unknown resource type returns 500.

After: unknown resource type returns 400 with the existing message.

Valid resource types are unchanged.

Any related issues, documentation, discussions?

Closes #8161

How was this PR tested?

Focused tests:

sbt -no-colors "WorkflowExecutionService/testOnly org.apache.texera.web.resource.dashboard.DashboardResourceSpec"

Result: 7 tests passed.

Checks:

sbt -no-colors scalafmtCheckAll

sbt -no-colors "scalafixAll --check"

Localhost verification on the built service:

GET /api/dashboard/publicSearch?resourceType=workflow returned 200.

GET /api/dashboard/publicSearch?resourceType=not-a-resource returned 400 with Unknown resource type: not-a-resource. The same request returned 500 before the change.

Was this PR authored or co-authored using generative AI tooling?

Generated-by: Codex GPT-5

@Yicong-Huang Yicong-Huang added the release/v1.2 back porting to release/v1.2 label Aug 30, 2026
@github-actions
github-actions Bot requested a review from xuang7 August 30, 2026 23:26
@github-actions

Copy link
Copy Markdown
Contributor

Backport auto-label report

This fix: PR was checked against each actively-supported release branch. release/* labels drive the post-merge backport, so add or remove one to change where this fix lands.

Release branch Analysis
⚠️ release/v1.3 Not labeled automatically — none of the files this PR modifies exist on this branch (amber/src/main/scala/org/apache/texera/web/resource/dashboard/DashboardResource.scala, amber/src/test/scala/org/apache/texera/web/resource/dashboard/DashboardResourceSpec.scala). The fix may target code that isn't on this release, or the files were moved/renamed after the branch was cut. Please check and add release/v1.3 by hand if this fix should be backported here.
release/v1.2 Change detected on this branch — label added; this fix is queued to backport here. Requested review from @xuang7.

Auto-label run.

@github-actions

Copy link
Copy Markdown
Contributor

Automated Reviewer Suggestions

Based on the git blame history of the changed files, we recommend the following reviewers:

  • Contributors with relevant context: @Neilk1021, @aglinxinyuan
    You can notify them by mentioning @Neilk1021, @aglinxinyuan in a comment.

@github-actions

github-actions Bot commented Aug 30, 2026

Copy link
Copy Markdown
Contributor

⚠️ Benchmark changes need a look

🟢 2 better · 🔴 8 worse · ⚪ 5 noise (<±5%) · 0 without baseline

Compared against main 70c2114 benchmarked on this same runner, so the delta is largely free of cross-runner hardware noise. The "7d avg" column still reflects the gh-pages dashboard. Treat <±5% as noise unless repeated.

Dashboard · Run

config throughput MB/s latency max Δ latest / 7d
🔴 bs=10 sw=10 sl=64 392 0.239 24,926/30,140/30,140 us 🟢 -14.6% / 🔴 +97.9%
🔴 bs=100 sw=10 sl=64 779 0.475 125,092/173,495/173,495 us 🔴 +17.4% / 🔴 +61.8%
bs=1000 sw=10 sl=64 904 0.552 1,100,815/1,164,866/1,164,866 us ⚪ within ±5% / 🔴 +13.9%
Baseline details

Latest main 70c2114 from same runner

config metric PR latest main 7d avg Δ latest Δ 7d
bs=10 sw=10 sl=64 throughput 392 tuples/sec 423 tuples/sec 787.98 tuples/sec -7.3% -50.3%
bs=10 sw=10 sl=64 MB/s 0.239 MB/s 0.258 MB/s 0.481 MB/s -7.4% -50.3%
bs=10 sw=10 sl=64 p50 24,926 us 22,846 us 12,593 us +9.1% +97.9%
bs=10 sw=10 sl=64 p95 30,140 us 35,274 us 15,579 us -14.6% +93.5%
bs=10 sw=10 sl=64 p99 30,140 us 35,274 us 18,786 us -14.6% +60.4%
bs=100 sw=10 sl=64 throughput 779 tuples/sec 829 tuples/sec 1,008 tuples/sec -6.0% -22.7%
bs=100 sw=10 sl=64 MB/s 0.475 MB/s 0.506 MB/s 0.615 MB/s -6.1% -22.8%
bs=100 sw=10 sl=64 p50 125,092 us 118,892 us 100,701 us +5.2% +24.2%
bs=100 sw=10 sl=64 p95 173,495 us 147,767 us 107,244 us +17.4% +61.8%
bs=100 sw=10 sl=64 p99 173,495 us 147,767 us 116,122 us +17.4% +49.4%
bs=1000 sw=10 sl=64 throughput 904 tuples/sec 914 tuples/sec 1,042 tuples/sec -1.1% -13.2%
bs=1000 sw=10 sl=64 MB/s 0.552 MB/s 0.558 MB/s 0.636 MB/s -1.1% -13.2%
bs=1000 sw=10 sl=64 p50 1,100,815 us 1,088,480 us 981,959 us +1.1% +12.1%
bs=1000 sw=10 sl=64 p95 1,164,866 us 1,193,973 us 1,023,080 us -2.4% +13.9%
bs=1000 sw=10 sl=64 p99 1,164,866 us 1,193,973 us 1,051,697 us -2.4% +10.8%
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,509.96,200,128000,392,0.239,24925.70,30140.25,30140.25
1,100,10,64,20,2568.29,2000,1280000,779,0.475,125092.19,173495.41,173495.41
2,1000,10,64,20,22127.33,20000,12800000,904,0.552,1100815.04,1164866.27,1164866.27

@codecov-commenter

codecov-commenter commented Aug 30, 2026

Copy link
Copy Markdown

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 93.96%. Comparing base (70c2114) to head (a47ec1a).
⚠️ Report is 14 commits behind head on main.
✅ All tests successful. No failed tests found.

Additional details and impacted files
@@             Coverage Diff              @@
##               main    #8162      +/-   ##
============================================
+ Coverage     93.94%   93.96%   +0.02%     
- Complexity     4759     4766       +7     
============================================
  Files          1185     1185              
  Lines         48069    48097      +28     
  Branches       5359     5361       +2     
============================================
+ Hits          45159    45195      +36     
+ Misses         1480     1476       -4     
+ Partials       1430     1426       -4     
Flag Coverage Δ *Carryforward flag
access-control-service 81.00% <ø> (ø) Carriedforward from 70c2114
agent-service 99.32% <ø> (ø) Carriedforward from 70c2114
amber 90.05% <100.00%> (+0.06%) ⬆️
computing-unit-managing-service 73.67% <ø> (ø) Carriedforward from 70c2114
config-service 86.86% <ø> (ø) Carriedforward from 70c2114
file-service 87.91% <ø> (ø) Carriedforward from 70c2114
frontend 96.49% <ø> (ø) Carriedforward from 70c2114
notebook-migration-service 79.31% <ø> (ø) Carriedforward from 70c2114
pyamber 98.87% <ø> (ø) Carriedforward from 70c2114
workflow-compiling-service 77.19% <ø> (ø) Carriedforward from 70c2114

*This pull request uses carry forward flags. Click here to find out more.

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

🚀 New features to boost your workflow:
  • 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

@carloea2 carloea2 changed the title fix(amber): reject invalid dashboard resource type feat(amber): reject invalid dashboard resource type Aug 31, 2026
@mengw15 mengw15 removed the release/v1.2 back porting to release/v1.2 label Aug 31, 2026
@xuang7 xuang7 removed the fix label Sep 1, 2026
@xuang7
xuang7 removed their request for review September 1, 2026 01:47
@carloea2

carloea2 commented Sep 4, 2026

Copy link
Copy Markdown
Contributor Author

Closing as a duplicate of #8144, which contains the same fix and regression coverage and is already approved.

@carloea2 carloea2 closed this Sep 4, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Unknown dashboard resource type returns server error

5 participants