Repository navigation
Commit 71bd0f4
authored
fix(trino): resolve the mapped host port in get_connection_url() (#1112)
Fixes #1111
### What's wrong
`TrinoContainer.get_connection_url()` builds the URL as
`trino://{user}@{host}:{self.port}`, and `self.port` is the fixed
container-internal port set in `__init__` (8080 by default) — never
resolved through `get_exposed_port()`. Since Docker maps that to a
random host port, the URL this method returns can't actually connect to
anything.
The existing test (`test_docker_run_trino`) didn't catch this because it
bypasses `get_connection_url()` entirely, connecting via
`trino.dbapi.connect()` with an explicitly resolved
`get_exposed_port(trino.port)` instead.
### The fix
One-line change, matching the pattern `CockroachDBContainer` and
`CrateDBContainer` already use via `_create_connection_url()`:
```diff
def get_connection_url(self):
- return f"trino://{self.user}@{self.get_container_host_ip()}:{self.port}"
+ return f"trino://{self.user}@{self.get_container_host_ip()}:{self.get_exposed_port(self.port)}"
```
Also added `test_get_connection_url`, which actually exercises
`get_connection_url()` via SQLAlchemy (the way a real user would use
it), matching the sibling `cockroachdb`/`cratedb` tests' pattern. Ran
locally against a real container — passes with the fix, and the bug is
reproducible by inspection: `trino.port` and
`trino.get_exposed_port(trino.port)` return different values whenever
Docker doesn't happen to map the container port 1:1 to the host, which
is the common case.
### Where this was found
Building a nightly CI job in `apache/superset` that tests its Trino
`db_engine_specs` module against a real Trino instance via this library
— more context in the linked issue.
# PR Checklist
- [x] PR title follows Conventional Commits (`fix(trino):`), scoped to
the community module per the community-modules note in the PR template.
- [x] Maintainer edits are allowed on this branch.
- [x] Change is in `src/testcontainers/community/trino/` with a
corresponding test in `tests/community/trino/`.
- [x] No new dependencies added.
- [x] Branch is up to date with `main`.1 parent 8d81011 commit 71bd0f4
2 files changed
Lines changed: 11 additions & 1 deletion
| Original file line number | Diff line number | Diff line change | |
|---|---|---|---|
| |||
38 | 38 | | |
39 | 39 | | |
40 | 40 | | |
41 | | - | |
| 41 | + | |
42 | 42 | | |
43 | 43 | | |
44 | 44 | | |
| Original file line number | Diff line number | Diff line change | |
|---|---|---|---|
| |||
| 1 | + | |
1 | 2 | | |
2 | 3 | | |
3 | 4 | | |
| |||
16 | 17 | | |
17 | 18 | | |
18 | 19 | | |
| 20 | + | |
| 21 | + | |
| 22 | + | |
| 23 | + | |
| 24 | + | |
| 25 | + | |
| 26 | + | |
| 27 | + | |
| 28 | + | |
0 commit comments