fix(playground): repair local compose config; prettier test_playground.sh output#4352
fix(playground): repair local compose config; prettier test_playground.sh output#4352igorroncevic merged 7 commits intomainfrom
test_playground.sh output#4352Conversation
|
All contributors have signed the CLA ✍️ ✅ |
|
I have read the CLA Document and I hereby sign the CLA |
There was a problem hiding this comment.
Code Review
This pull request updates configurations for autopilot, driver, and orderbook services, including database URLs, ethflow settings, and port adjustments. It also refines the Tempo storage setup and enhances the test_playground.sh script with improved status polling and settlement transaction reporting. Feedback focuses on ensuring shell script portability by replacing echo -e with printf and restoring robustness to curl commands by re-adding retry flags in the polling loop.
jmg-duarte
left a comment
There was a problem hiding this comment.
config changes look ok but i have some questions
|
|
||
| # Setup parameters | ||
| HOST=localhost:8080 | ||
| OTTERSCAN_URL=${OTTERSCAN_URL:-http://localhost:8003} |
There was a problem hiding this comment.
isnt this set by default in the dockerfile + docker compose?
There was a problem hiding this comment.
Compose defines the port mapping, but it does not export OTTERSCAN_URL into the host shell.
The script runs on the host, so if we want a configurable value here we need to set it in the script or require the user to export it.
There was a problem hiding this comment.
but the script doesnt care for otterscan, the test_playground is something one off
and writing it here doesnt export is to the host shell either, scripts spawn they own bash shells so when they finish the variables basically get wipped
There was a problem hiding this comment.
Fair point, I'll simply hardcode it to http://localhost:8003 so it's easier to find
| - db-migrations | ||
| ports: | ||
| - 8080:80 # API | ||
| - 8080:8080 # API |
There was a problem hiding this comment.
is this really necessary?
There was a problem hiding this comment.
I'm seeing that orderbook launches on 8080 internally, so shouldn't it be necessary?
There was a problem hiding this comment.
that's the default, the change above maps 8080 on docker to 80 on host, which should allow you to write localhost instead of localhost:8080
There was a problem hiding this comment.
Isn't it viceversa? host:container? so the second 8080 relates to container port which orderbook currently launches on?
There was a problem hiding this comment.
but the orderbook config binds it to 80: https://github.com/cowprotocol/services/blob/main/playground/orderbook.toml#L2
There was a problem hiding this comment.
ah, I see the issue. playground/docker-compose.fork.yml:
- uses
playground/**_configs_**/orderbook.tomlas config - and not
playground/orderbook.tomlthat you shared
Looks like that's a leftover from before, so I'd like to just delete playground/orderbook.toml and fix playground/configs/orderbook.toml to use port 80. Sounds good?
There was a problem hiding this comment.
Yeah, lets make this playground/configs/orderbook.toml be the only one left
You can also delete configs/local/orderbook.toml
| write-url = "%DB_WRITE_URL" | ||
| read-url = "%DB_READ_URL" | ||
|
|
||
| [ethflow] |
There was a problem hiding this comment.
I don't think this change should be necessary?
There was a problem hiding this comment.
I'm seeing here that it's a required value and it gave me errors without it. Perhaps I'm wrong but it seemed like it just didn't have that value.
There was a problem hiding this comment.
this doesn't hurt being here, but it shouldnt be required, it just wont index ethflow trades
There was a problem hiding this comment.
test_playground actually creates an EthFlow order, so this should be here because of it.
df44f2a to
d679bc7
Compare
Description
Fix the local playground setup so the Docker stack starts cleanly and
playground/test_playground.shworks end to end again.This was needed because several local-only config mismatches had accumulated:
tempowas crashing on startup becauseplayground/tempo.yamlused old config fields that the currentgrafana/tempo:latestimage no longer acceptsorderbookwas not reachable correctly because wrong config was being loadedautopilotwas missing the local database and EthFlow config needed to pick up and process the onchain test orderdriverwas trying to reachorderbookon the wrong internal portLess importantly,
test_playground.shhad rough edges in its output and did not print the settlement transaction hash after a successful run.Error details seen locally
These are the main errors that showed up while debugging:
1. Tempo / tracing startup failure
driverwas logging OpenTelemetry export failures like:This looked like DNS, but the real issue was that
temponever came up because its config was invalid for the current image.2. Early readiness / connection reset failures
Running the playground test initially failed during readiness checks with:
This came from
orderbooknot starting correctly and from an incorrect config being passed to it.3. Autopilot startup crash
Before the local config was fixed,
autopilotcrashed during startup with a panic around:This was caused by missing database configuration in the local playground config.
4. Driver could not talk to orderbook internally
When orders started reaching the solver path,
driverstill failed to fetch app data because it was callingorderbookon the wrong internal port due to the wrong config:5. Solve request timing mismatch
autopilotwould submit a solve request,driverwould compute and score a valid settlement, butautopilotwould still time out waiting for the response. The visible symptom was:The root cause was that the default local solve deadline was too long for this playground test flow, so
driverkept the solve request open much longer than the script expected.Changes
playground/tempo.yamlto the config format accepted by the current Tempo imageplayground/configs/orderbook.tomlorderbookconfig being passed to itplayground/configs/autopilot.tomlplayground/configs/autopilot.tomlsolve-deadlineinplayground/configs/autopilot.tomlso the playground test does not wait on production-style timingtest_playground.shplayground/test_playground.shto print the settlement transaction hash after successplayground/test_playground.shto print a ready-to-runcast receiptcommand and an Otterscan transaction URLplayground/test_playground.shstatus output so expected pre-indexing404responses are shown asindexingHow to test
docker compose -f playground/docker-compose.fork.yml up -dcd playground && ./test_playground.shtradedcast receipt ... --rpc-url http://localhost:8545commandcast receiptcommand or by opening the printed Otterscan URL