refactor(config): unify default-data-transfer-batch-size into one config key#5545
refactor(config): unify default-data-transfer-batch-size into one config key#5545Ma77Ball wants to merge 5 commits into
Conversation
Codecov Report❌ Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #5545 +/- ##
============================================
- Coverage 52.38% 52.36% -0.02%
+ Complexity 2484 2479 -5
============================================
Files 1070 1070
Lines 41359 41361 +2
Branches 4441 4441
============================================
- Hits 21666 21660 -6
+ Misses 18427 18426 -1
- Partials 1266 1275 +9
*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:
|
| @GET | ||
| @RolesAllowed(Array("REGULAR", "ADMIN")) | ||
| @Path("/gui") | ||
| def getGuiConfig: Map[String, Any] = |
There was a problem hiding this comment.
let's not put engine configs under gui. Please create a new endpoint for engine configs.
e.g, /amber
There was a problem hiding this comment.
Moved defaultDataTransferBatchSize out of /gui into a new /amber endpoint, and updated the frontend to fetch it there.
| import jakarta.annotation.security.{PermitAll, RolesAllowed} | ||
| import jakarta.ws.rs.core.MediaType | ||
| import jakarta.ws.rs.{GET, Path, Produces} | ||
| import org.apache.texera.amber.config.ApplicationConfig |
There was a problem hiding this comment.
config service should not depend on amber. we need to separate them. otherwise config service will require to be compiled with amber package.
There was a problem hiding this comment.
ApplicationConfig actually lives in common/config, not the amber module, so config-service does not depend on amber (the package is just named amber). I added a short comment at the import to make that clear. Let me know if you'd still prefer it organized differently.
3 participants
What changes were proposed in this PR?
The default data-transfer batch size was controlled by two separate config keys, each with its own env var, both defaulting to
400:network-buffering.default-data-transfer-batch-size(NETWORK_BUFFERING_DEFAULT_DATA_TRANSFER_BATCH_SIZE), read by the backend (ApplicationConfig.defaultDataTransferBatchSize).gui.workflow-workspace.default-data-transfer-batch-size(GUI_WORKFLOW_WORKSPACE_DEFAULT_DATA_TRANSFER_BATCH_SIZE), read byGuiConfigand sent to the frontend viaConfigResource.Having two keys for one value meant an operator could set one env var and forget the other, leaving the backend and GUI out of sync.
This PR makes the
network-bufferingkey the single source of truth:ConfigResourcenow surfacesApplicationConfig.defaultDataTransferBatchSizedirectly.guiWorkflowWorkspaceDefaultDataTransferBatchSizefield fromGuiConfig.gui.conf.The frontend is unchanged: the JSON field name (
defaultDataTransferBatchSize) it consumes stays the same.Any related issues, documentation, discussions?
Closes #5544
How was this PR tested?
sbt Config/compile ConfigService/compileboth succeed.GUI_WORKFLOW_WORKSPACE_DEFAULT_DATA_TRANSFER_BATCH_SIZE(outside generateddist/artifacts).NetworkOutputBufferSpec) are unaffected.Was this PR authored or co-authored using generative AI tooling?
Generated-by: Claude Code (Opus 4.8)