aicam commented on code in PR #6858:
URL: https://github.com/apache/texera/pull/6858#discussion_r3825141556
##########
common/config/src/main/resources/application.conf:
##########
@@ -90,7 +90,7 @@ web-server {
clean-all-execution-results-on-server-start = false
clean-all-execution-results-on-server-start =
${?WEB_SERVER_CLEAN_ALL_EXECUTION_RESULTS_ON_SERVER_START}
- max-workflow-websocket-request-payload-size-kb = 64
+ max-workflow-websocket-request-payload-size-kb = 1024
Review Comment:
Good catch, fixed in `b558dfe4e`. `ApplicationConfigSpec.scala:134` now
asserts `shouldBe 1024`, and `build / amber (ubuntu-latest, 17)` is green again
— the "Run amber and common module tests with coverage" step passes.
I took the other two findings in the same commit: `values-development.yaml`
now has `"1024"` so the dev chart no longer reproduces #6857, and the `// by
default, 64 KiB` comment on `WebsocketPayloadSizeTuner` is updated. The PR
description is corrected to the current template heading and lists all four
places the default is encoded.
Your explanation of why the failure isn't visible from the checks list is
right, and I confirmed it: `Config/jacoco` appears only in the amber job's sbt
invocation (`build.yml:310`), while `platform (config-service)` runs
`ConfigService/jacoco` — a separate project (`build.sbt:121` `Config` in
`common/config` vs `build.sbt:128` `ConfigService` in `config-service`).
`ConfigService` merely `dependsOn(Config)`, which pulls in the compiled
classes, not the test suite, so `ApplicationConfigSpec` never runs there. That
means any breakage in `common/config` tests surfaces only under the amber job,
which reads as an unrelated area. Worth a separate issue to give the common
modules their own check rather than burying them in amber — happy to file one
if you agree.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]