testcontainers / testcontainers-python

Testcontainers is a Python library that providing a friendly API to run Docker container. It is designed to create runtime environment to use during your automatic tests.
https://testcontainers-python.readthedocs.io/en/latest/
Apache License 2.0
1.51k stars 281 forks source link

Set elasticseach http.port environment variable for non default ports #593

Closed joeyjurjens closed 3 months ago

joeyjurjens commented 3 months ago

I tried running test containers for a project of mine, since I already had a local instance running on 9200 (default port) I set a custom port. However, the tests seemed to fail because it couldn't connect to the instance. This is because ES expects to run on 9200, unless you set the http.port setting (which you can with an environment variable).

So I thought it might be useful to do that by default, so others don't have to debug this issue themselves.

alexanderankin commented 3 months ago

that's right, the port that you set on the constructor is the port inside the container. in order to get the port outside the container, you need to use this method:

https://github.com/testcontainers/testcontainers-python/blob/a95af7ddab9de5d375031a9071d6a680e47ba981/core/testcontainers/core/container.py#L140-L149

alexanderankin commented 3 months ago

so if you have something already running on your pc on port 9200, e.g.:

python -m http.server 9200

and then you start testcontainers:

with ElasticSearchContainer() as container:
  print(container.get_exposed_port(9200))  # will not print 9200!

elastic search will occupy some random port when it runs, not 9200.

joeyjurjens commented 3 months ago

so if you have something already running on your pc on port 9200, e.g.:

python -m http.server 9200

and then you start testcontainers:

with ElasticSearchContainer() as container:
  print(container.get_exposed_port(9200))  # will not print 9200!

elastic search will occupy some random port when it runs, not 9200.

Right, I saw that as get_url also uses that method (which I use in my tests). However the ElasticSearchContainer class accepts a port argument, and when you pass a port other than 9200, you can't make a connection to it unless you set the http.port env variable. So I thought I'd fix that in case others, like me, were also passing a different port, as in the end it is made available to be changed.

alexanderankin commented 3 months ago

i reviewed the java implementation this morning and it doesn't provide a mechanism to change the port. i cannot figure out why this would be necessary either. I would merge a PR that removes all the superfluous "port" arguments, there's quite a bunch and you are correct that it does cause friction for newcomers

joeyjurjens commented 3 months ago

Alright, I might give that a go if that would help you.