Skip to content

refactor(frontend): preserve config source in GuiConfigService instead of one flat map#5673

Draft
Ma77Ball wants to merge 11 commits into
apache:mainfrom
Ma77Ball:isolate-config-sources-5669
Draft

refactor(frontend): preserve config source in GuiConfigService instead of one flat map#5673
Ma77Ball wants to merge 11 commits into
apache:mainfrom
Ma77Ball:isolate-config-sources-5669

Conversation

@Ma77Ball

Copy link
Copy Markdown
Contributor

Stacked on #5545 (adds the /config/amber endpoint and the amber config source this PR namespaces). Base diff includes #5545 until it merges; review the last commit only, or after #5545 lands. Kept as a draft for that reason.

What changes were proposed in this PR?

  • Changed GuiConfigService to store each endpoint's payload under its own key in a configBySource map (preLogin, gui, amber, userSystem) instead of spreading them all into one flat object, so the origin of every config value is preserved (addresses the review comment on refactor(config): unify default-data-transfer-batch-size into one config key #5545).
  • Derived the existing flat env getter from configBySource on read, keeping its shape identical so the ~50 existing guiConfigService.env.<flag> call sites are untouched.
  • Added a source(name) accessor that returns one endpoint's payload in isolation.
  • Added unit tests for the isolation guarantees (each source retrievable alone, no key bleed across sources, empty object for an unloaded source).

Any related issues, documentation, discussions?

Closes: #5669

How was this PR tested?

  • Run cd frontend && npx ng test --include="src/app/common/service/gui-config.service.spec.ts" and expect 11 passed (8 existing merge/orchestration tests plus 3 new source-isolation tests).
  • Confirm the merged view is unchanged: service.env.defaultDataTransferBatchSize still resolves after post-login load, while service.source("amber") returns only the amber payload and service.source("gui") does not contain defaultDataTransferBatchSize.

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

Co-authored with Claude Opus 4.8 in compliance with ASF

@github-actions github-actions Bot added frontend Changes related to the frontend GUI common platform Non-amber Scala service paths labels Jun 13, 2026
@codecov-commenter

codecov-commenter commented Jun 13, 2026

Copy link
Copy Markdown

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 53.04%. Comparing base (7ae9b35) to head (6b68a32).

Additional details and impacted files
@@             Coverage Diff              @@
##               main    #5673      +/-   ##
============================================
- Coverage     53.12%   53.04%   -0.09%     
- Complexity     2544     2545       +1     
============================================
  Files          1075     1075              
  Lines         42232    41743     -489     
  Branches       4604     4514      -90     
============================================
- Hits          22436    22141     -295     
+ Misses        18480    18293     -187     
+ Partials       1316     1309       -7     
Flag Coverage Δ *Carryforward flag
access-control-service 71.42% <ø> (-3.36%) ⬇️
agent-service 34.36% <ø> (ø) Carriedforward from a4153ca
amber 53.93% <ø> (+0.01%) ⬆️
computing-unit-managing-service 1.65% <ø> (ø)
config-service 57.35% <100.00%> (-1.22%) ⬇️
file-service 57.06% <ø> (ø)
frontend 47.42% <100.00%> (-0.08%) ⬇️
pyamber 90.70% <ø> (-0.09%) ⬇️ Carriedforward from a4153ca
python 90.74% <ø> (ø) Carriedforward from a4153ca
workflow-compiling-service 58.69% <ø> (ø)

*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.

@github-actions

Copy link
Copy Markdown
Contributor

✅ No material benchmark regressions detected

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

CI benchmark results are noisy; treat <±5% as noise unless repeated.

Dashboard · Run

config throughput MB/s latency max Δ latest / 7d
🟢 bs=10 sw=10 sl=64 390 0.238 25,233/35,168/35,168 us 🟢 -8.3% / ⚪ within ±5%
🟢 bs=100 sw=10 sl=64 919 0.561 105,293/133,992/133,992 us 🟢 +26.1% / 🟢 -8.3%
🟢 bs=1000 sw=10 sl=64 1,118 0.682 894,696/941,525/941,525 us 🟢 +18.1% / 🟢 +10.7%
Baseline details

Latest main 7ae9b35 from 2026-06-13T00:44:28.840Z

config metric PR latest main 7d avg Δ latest Δ 7d
bs=10 sw=10 sl=64 throughput 390 tuples/sec 394.18 tuples/sec 400.34 tuples/sec -1.1% -2.6%
bs=10 sw=10 sl=64 MB/s 0.238 MB/s 0.241 MB/s 0.244 MB/s -1.1% -2.6%
bs=10 sw=10 sl=64 p50 25,233 us 24,805 us 24,191 us +1.7% +4.3%
bs=10 sw=10 sl=64 p95 35,168 us 38,345 us 36,018 us -8.3% -2.4%
bs=10 sw=10 sl=64 p99 35,168 us 38,345 us 36,018 us -8.3% -2.4%
bs=100 sw=10 sl=64 throughput 919 tuples/sec 729.17 tuples/sec 867.19 tuples/sec +26.0% +6.0%
bs=100 sw=10 sl=64 MB/s 0.561 MB/s 0.445 MB/s 0.529 MB/s +26.1% +6.0%
bs=100 sw=10 sl=64 p50 105,293 us 133,219 us 114,874 us -21.0% -8.3%
bs=100 sw=10 sl=64 p95 133,992 us 173,178 us 142,264 us -22.6% -5.8%
bs=100 sw=10 sl=64 p99 133,992 us 173,178 us 142,264 us -22.6% -5.8%
bs=1000 sw=10 sl=64 throughput 1,118 tuples/sec 946.3 tuples/sec 1,010 tuples/sec +18.1% +10.7%
bs=1000 sw=10 sl=64 MB/s 0.682 MB/s 0.578 MB/s 0.616 MB/s +18.1% +10.7%
bs=1000 sw=10 sl=64 p50 894,696 us 1,059,718 us 995,071 us -15.6% -10.1%
bs=1000 sw=10 sl=64 p95 941,525 us 1,104,221 us 1,046,228 us -14.7% -10.0%
bs=1000 sw=10 sl=64 p99 941,525 us 1,104,221 us 1,046,228 us -14.7% -10.0%
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,512.23,200,128000,390,0.238,25233.13,35167.89,35167.89
1,100,10,64,20,2176.11,2000,1280000,919,0.561,105292.72,133991.67,133991.67
2,1000,10,64,20,17885.95,20000,12800000,1118,0.682,894695.81,941525.09,941525.09

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

common frontend Changes related to the frontend GUI platform Non-amber Scala service paths

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Preserve config source isolation in GuiConfigService instead of merging all endpoints into one flat map

2 participants