Skip to content

fix(trino): resolve the mapped host port in get_connection_url() - #1112

Open
rusackas wants to merge 1 commit into
testcontainers:mainfrom
rusackas:fix/trino-connection-url-port
Open

fix(trino): resolve the mapped host port in get_connection_url()#1112
rusackas wants to merge 1 commit into
testcontainers:mainfrom
rusackas:fix/trino-connection-url-port

Conversation

@rusackas

@rusackas rusackas commented Aug 26, 2026

Copy link
Copy Markdown

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():

     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

  • PR title follows Conventional Commits (fix(trino):), scoped to the community module per the community-modules note in the PR template.
  • Maintainer edits are allowed on this branch.
  • Change is in src/testcontainers/community/trino/ with a corresponding test in tests/community/trino/.
  • No new dependencies added.
  • Branch is up to date with main.

get_connection_url() returned the container-internal port (self.port,
e.g. 8080) instead of the Docker-mapped host port, so the URL it built
could never actually connect -- the existing test bypassed this method
entirely, connecting via trino.dbapi.connect() with an explicitly
resolved get_exposed_port() instead, so the bug went uncaught.

Fixes the same way CockroachDBContainer/CrateDBContainer already do it
via _create_connection_url(), and adds a test that actually exercises
get_connection_url() via SQLAlchemy, matching the sibling cockroachdb/
cratedb tests' pattern.

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

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

TrinoContainer.get_connection_url() returns the internal port, not the mapped host port

1 participant