| FazBrowse GitHub Viewer | Trending | | Home |
| Tools: [Download Repo ZIP] [Original HTTPS Page] |
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -1,16 +1,29 @@ | ||
| ARG PYTHON_VERSION | ||
| FROM python:${version}-slim-bookworm | ||
| ARG PYTHON_VERSION=3.10 | ||
| FROM python:${PYTHON_VERSION}-slim-bookworm | ||
|
|
||
| ENV POETRY_NO_INTERACTION=1 \ | ||
| POETRY_VIRTUALENVS_IN_PROJECT=1 \ | ||
| POETRY_VIRTUALENVS_CREATE=1 \ | ||
| POETRY_CACHE_DIR=/tmp/poetry_cache | ||
|
|
||
| WORKDIR /workspace | ||
| RUN pip install --upgrade pip \ | ||
| && apt-get update \ | ||
| && apt-get install -y \ | ||
| freetds-dev \ | ||
| && rm -rf /var/lib/apt/lists/* | ||
| && apt-get install -y freetds-dev \ | ||
| && apt-get install -y make \ | ||
| # no real need for keeping this image small at the moment | ||
| && :; # rm -rf /var/lib/apt/lists/* | ||
|
|
||
| # install poetry | ||
| RUN bash -c 'python -m venv /opt/poetry-venv && source $_/bin/activate && pip install poetry && ln -s $(which poetry) /usr/bin' | ||
|
|
||
| # install requirements we exported from poetry | ||
| COPY build/requirements.txt requirements.txt | ||
| RUN pip install -r requirements.txt | ||
| # install dependencies with poetry | ||
| COPY pyproject.toml . | ||
| COPY poetry.lock . | ||
| RUN poetry install --all-extras --with dev --no-root | ||
|
|
||
| # copy project source | ||
| COPY . . | ||
|
|
||
| # install project with poetry | ||
| RUN poetry install --all-extras --with dev |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -1,6 +1,35 @@ | ||
| from pathlib import Path | ||
|
|
||
| import pytest | ||
| from typing import Callable | ||
| import subprocess | ||
| from testcontainers.core.container import DockerClient | ||
| import sys | ||
|
|
||
| PROJECT_DIR = Path(__file__).parent.parent.parent.resolve() | ||
|
|
||
|
|
||
| def pytest_configure(config: pytest.Config) -> None: | ||
| """ | ||
| Add configuration for custom pytest markers. | ||
| """ | ||
| config.addinivalue_line( | ||
| "markers", | ||
| "inside_docker_check: test used to validate DinD/DooD are working as expected", | ||
| ) | ||
|
Comment thread
Comment on lines
+12
to
+19
Copy link
Copy Markdown
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Choose a reason Spam Abuse Off Topic Outdated Duplicate Resolved Low Qualityis this just for filtering in/out dind/dod tests?
Sorry, something went wrong.
All reactions
Copy link
Copy Markdown
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Choose a reason Spam Abuse Off Topic Outdated Duplicate Resolved Low QualityThat is to filter out tests that run within the test_dind and test_dood tests. I left the old make test-dind (which is not correct because it's acutall DooD) untouched.
Sorry, something went wrong.
All reactions
Copy link
Copy Markdown
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Choose a reason Spam Abuse Off Topic Outdated Duplicate Resolved Low Qualitylets remove the things that don't make sense
Sorry, something went wrong.
All reactions
Copy link
Copy Markdown
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Choose a reason Spam Abuse Off Topic Outdated Duplicate Resolved Low Qualityperhaps in a separate PR maybe to make the history clearer
Sorry, something went wrong.
All reactions
|
||
|
|
||
|
|
||
| @pytest.fixture(scope="session") | ||
| def python_testcontainer_image() -> str: | ||
| """Build an image with test containers python for DinD and DooD tests""" | ||
| py_version = ".".join(map(str, sys.version_info[:2])) | ||
| image_name = f"testcontainers-python:{py_version}" | ||
| subprocess.run( | ||
| [*("docker", "build"), *("--build-arg", f"PYTHON_VERSION={py_version}"), *("-t", image_name), "."], | ||
|
Comment thread
Copy link
Copy Markdown
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Choose a reason Spam Abuse Off Topic Outdated Duplicate Resolved Low Quality@CarliJoy @alexanderankin
Sorry, something went wrong.
All reactions
Copy link
Copy Markdown
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Choose a reason Spam Abuse Off Topic Outdated Duplicate Resolved Low QualityThen is still as good as before, as the Makefile also was using sudoless docker. Only difference people will know that there is an issue because before they probably never cared to run make test-dind. Or they will create an Issue at which point we can take care of this.
Sorry, something went wrong.
All reactions
Copy link
Copy Markdown
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Choose a reason Spam Abuse Off Topic Outdated Duplicate Resolved Low Qualitycool, let me try it on a vm :)
Sorry, something went wrong.
All reactions
Copy link
Copy Markdown
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Choose a reason Spam Abuse Off Topic Outdated Duplicate Resolved Low QualityOr put in other words: This is only relevant for testing and as long as people who want to contribute don't run into problems and it works in the pipeline I would not invest time into this.
Sorry, something went wrong.
All reactions
Copy link
Copy Markdown
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Choose a reason Spam Abuse Off Topic Outdated Duplicate Resolved Low QualityI'm asking because I had some unrelated issues that made me rerun all the tests and now (once I fixed the other stuff) I'm left with this, in any case I'll try and recreate all the env as sudoless.
Sorry, something went wrong.
All reactions
Copy link
Copy Markdown
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Choose a reason Spam Abuse Off Topic Outdated Duplicate Resolved Low QualitySorry to hear.
Sorry, something went wrong.
All reactions
Copy link
Copy Markdown
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Choose a reason Spam Abuse Off Topic Outdated Duplicate Resolved Low Qualityok, so updating if anyone needs in the future, the workaround is very trivial just run with [*("sudo", "docker", "build"), *("--build-arg", f"PYTHON_VERSION={py_version}"), *("-t", image_name), "."],once and you should be good :) as the image will be created and available.
Sorry, something went wrong.
All reactions
Copy link
Copy Markdown
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Choose a reason Spam Abuse Off Topic Outdated Duplicate Resolved Low QualityNo not once -> everytime you update the code. If you already have the error in front of you, why not catch it and rerun it with sudo? First try without sudo and when running into the troubles, just add it. What do you think of this idea?
Sorry, something went wrong.
All reactions
Copy link
Copy Markdown
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Choose a reason Spam Abuse Off Topic Outdated Duplicate Resolved Low QualityCreated an issue to document this #749
Sorry, something went wrong.
All reactions
Copy link
Copy Markdown
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Choose a reason Spam Abuse Off Topic Outdated Duplicate Resolved Low QualityJust to clarify, once is just to get the tests working, if I don't change anything dind related I tend to trust the CI for those kind of changes, but as a safety net I'll try and move my env to use sudoless. The correct approach in my options is to stop using subprocess.run for docker related activity, as we have the docker SDK :) I also noted this in the ticket for the long run.
Sorry, something went wrong.
All reactions
|
||
| cwd=PROJECT_DIR, | ||
| check=True, | ||
| ) | ||
| return image_name | ||
|
|
||
|
|
||
| @pytest.fixture | ||
| Expand Down | ||
| Back | FazBrowse Home | New Git URL |
Uh oh!
There was an error while loading. Please reload this page.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Choose a reason Spam Abuse Off Topic Outdated Duplicate Resolved Low QualityBtw: one could argue that this is incorrect.
self.get_docker_client().host() does return a DNS name instead of an IP especially localhost.
Indeed for some containers this has a negative effect: I tried to create a MariaDB container that failed. Using localhost instead of 127.0.0.1 it was trying to connect through the unix socket.
But this is the old behaviour so I kept it the way it is.
We might want to change this in the future.
Sorry, something went wrong.
Uh oh!
There was an error while loading. Please reload this page.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Choose a reason Spam Abuse Off Topic Outdated Duplicate Resolved Low Qualityah yes, as it says "container_ip" in the method name but it returns a domain name instead, hm.
Sorry, something went wrong.
Uh oh!
There was an error while loading. Please reload this page.