-
Notifications
You must be signed in to change notification settings - Fork 298
New issue
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
docs(contributing): add contribution and new-container guide #460
Merged
Merged
Changes from all commits
Commits
Show all changes
18 commits
Select commit
Hold shift + click to select a range
bc2cc40
chore(build): improve makefile
totallyzen 8fdff6c
fix: run pre-commit through poetry when calling through make
totallyzen 9ac3b08
docs(contributing): add CONTRIBUTING.md and draft of new-container.md
totallyzen 57017f9
nits
alexanderankin f860596
fix: typo in .github/CONTRIBUTING.md
totallyzen 15583e2
fix: groom question issue template
totallyzen f5e8e51
docs: add stuff on adding new containers
totallyzen ef90450
fix: clean up Makefile, make it more coherent, even if dind is still …
totallyzen cb77560
fix: nit wording
totallyzen 2b6fefe
fix: refer to make lint in contributing.md
totallyzen 8acd060
fix: small typo
totallyzen f7866a4
fix: go back to /doctests (in plural form)
totallyzen c823a35
fix: reference conventional commits as it's important to development
totallyzen 6466528
fix: remove setup.py references and refer to poetry
totallyzen 2778548
docs: document community module support facts
totallyzen 874f063
fix: add more warnings and explanation
totallyzen d55b6ec
docs: Added badges to README.md (#528)
max-pfeiffer 1276acb
replace child packages with extras notation for package structure sec…
alexanderankin File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,82 @@ | ||
# Contributing to `testcontainers-python` | ||
|
||
Welcome to the `testcontainers-python` community! | ||
This should give you an idea about how we build, test and release `testcontainers-python`! | ||
|
||
Highly recommended to read this document thoroughly to understand what we're working on right now | ||
and what our priorities are before you are trying to contribute something. | ||
|
||
This will greatly increase your chances of getting prompt replies as the maintainers are volunteers themselves. | ||
|
||
## Before you Begin | ||
|
||
We recommend following these steps: | ||
|
||
1. Finish reading this document. | ||
2. Read the [recently updated issues][1] | ||
3. Look for existing issues on the subject you are interested in - we do our best to label everything correctly | ||
|
||
|
||
## Local Development | ||
|
||
### Pre-Requisites | ||
|
||
You need to have the following tools available to you: | ||
- `make` - You'll need a GNU Make for common developer activities | ||
- `poetry` - This is the primary package manager for the project | ||
- `pyenv` **Recommended**: For installing python versions for your system. | ||
Poetry infers the current latest version from what it can find on the `PATH` so you are still fine if you don't use `pyenv`. | ||
|
||
### Build and test | ||
|
||
|
||
- Run `make install` to get `poetry` to install all dependencies and set up `pre-commit` | ||
totallyzen marked this conversation as resolved.
Show resolved
Hide resolved
|
||
- **Recommended**: Run `make` or `make help` to see other commands available to you. | ||
- After this, you should have a working virtual environment and proceed with writing code with your favourite IDE | ||
- **TIP**: You can run `make core/tests` or `make module/<my-module>/tests` to run the tests specifically for that to speed up feedback cycles | ||
- You can also run `make lint` to run the `pre-commit` for the entire codebase. | ||
|
||
|
||
## Adding new containers | ||
|
||
We have an [issue template](.github/ISSUE_TEMPLATE/new-container.md) for adding new containers, please refer to that for more information. | ||
Once you've talked to the maintainers (we do our best to reply!) then you can proceed with contributing the new container. | ||
|
||
> [!WARNING] | ||
> PLease raise an issue before you try to contribute a new container! It helps maintainers understand your use-case and motivation. | ||
> This way we can keep pull requests foruced on the "how", not the "why"! :pray: | ||
> It also gives maintainers a chance to give you last-minute guidance on caveats or expectations, particularly with | ||
> new extra dependencies and how to manage them. | ||
|
||
|
||
## Raising Issues | ||
|
||
We have [Issue Templates][2] to cover most cases, please try to adhere to them, they will guide you through the process. | ||
Try to look through the existing issues before you raise a new one. | ||
|
||
|
||
## Releasing Versions | ||
|
||
We have automated Semantic Versioning and release via [release-please](workflows/release-please.yml). | ||
This takes care of: | ||
- Detecting the next version, based on the commits that landed on `main` | ||
- When a Release PR has been merged | ||
- Create a GitHub Release with the CHANGELOG included | ||
- Update the [CHANGELOG](../CHANGELOG.md), similar to the GitHub Release | ||
- Release to PyPI via a [trusted publisher](https://docs.pypi.org/trusted-publishers/using-a-publisher/) | ||
- Automatically script updates in files where it's needed instead of hand-crafting it (i.e. in `pyproject.toml`) | ||
|
||
> [!CRITICAL] | ||
alexanderankin marked this conversation as resolved.
Show resolved
Hide resolved
|
||
> Community modules are supported on a best-effort basis and for maintenance reasons, any change to them | ||
> is only covered under minor and patch changes. | ||
> | ||
> Community modules changes DO NOT contribute to major version changes! | ||
> | ||
> If your community module container was broken by a minor or patch version change, check out the change logs! | ||
|
||
# Thank you! | ||
|
||
Thanks for reading, feedback on documentation is always welcome! | ||
|
||
[1]: https://github.com/testcontainers/testcontainers-python/issues?q=is%3Aissue+is%3Aopen+sort%3Aupdated-desc "Recently Updated Issues showing you what we're focusing on" | ||
[2]: https://github.com/testcontainers/testcontainers-python/issues/new/choose "List of current issue templates, please use them" |
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,26 @@ | ||
--- | ||
name: New Container | ||
about: Tell the Testcontainers-Python team about a container you'd like to have support for. | ||
title: 'New Container: ' | ||
labels: '🚀 enhancement' | ||
assignees: '' | ||
|
||
--- | ||
|
||
<!-- feel free to remove any irrelevant section(s) below --> | ||
|
||
**What is the new container you'd like to have?** | ||
|
||
Please link some docker containers as well as documentation/arguments to the benefits of having this container. | ||
|
||
**Why not just use a generic container for this?** | ||
|
||
Please describe why the `DockerContainer("my-image:latest")` approach is not useful enough. | ||
|
||
Having a dedicated `TestContainer` usually means the need for some or all of these: | ||
- complicated setup/configuration | ||
- the wait strategy is complex for the container, usually more than just an http wait | ||
|
||
**Other references:** | ||
|
||
Include any other relevant reading material about the enhancement. |
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -1,8 +1,40 @@ | ||
You have implemented a new container and would like to contribute it? Great! Here are the necessary steps. | ||
|
||
- [ ] Create a new feature directory and populate it with the package structure [described in the documentation](https://testcontainers-python.readthedocs.io/en/latest/#package-structure). Copying one of the existing features is likely the best way to get started. | ||
- [ ] Implement the new feature (typically in `__init__.py`) and corresponding tests. | ||
- [ ] Update the feature `README.rst` and add it to the table of contents (`toctree` directive) in the top-level `README.rst`. | ||
- [ ] Add a line `[feature name]` to the list of components in the GitHub Action workflow in `.github/workflows/main.yml` to run tests, build, and publish your package when pushed to the `main` branch. | ||
- [ ] Rebase your development branch on `main` (or merge `main` into your development branch). | ||
- [ ] Add a line `-e file:[feature name]` to `requirements.in` and open a pull request. Opening a pull request will automatically generate lock files to ensure reproducible builds (see the [pip-tools documentation](https://pip-tools.readthedocs.io/en/latest/) for details). Finally, run `python get_requirements.py --pr=[your PR number]` to fetch the updated requirement files (the build needs to have succeeded). | ||
# New Container | ||
|
||
<!-- You have implemented a new container and would like to contribute it? Great! Here are the necessary checklist steps. --> | ||
|
||
Fixes ... | ||
|
||
<!-- | ||
Please do not raise a PR for new container without having raised an issue first. | ||
It helps reduce unnecessary work for you and the maintainers! | ||
--> | ||
|
||
|
||
# PR Checklist | ||
|
||
- [ ] Your PR title follows the [Conventional Commits](https://www.conventionalcommits.org/en/v1.0.0/) syntax | ||
as we make use of this for detecting Semantic Versioning changes. | ||
- [ ] Your PR allows maintainers to edit your branch, this will speed up resolving minor issues! | ||
- [ ] The new container is implemented under `modules/*` | ||
- Your module follows [PEP 420](https://peps.python.org/pep-0420/) with implicit namespace packages | ||
(if unsure, look at other existing community modules) | ||
- Your package namespacing follows `testcontainers.<modulename>.*` | ||
and you DO NOT have an `__init__.py` above your module's level. | ||
- Your module has it's own tests under `modules/*/tests` | ||
- Your module has a `README.rst` and hooks in the `.. auto-class` and `.. title` of your container | ||
- Implement the new feature (typically in `__init__.py`) and corresponding tests. | ||
- [ ] Your module is added in `pyproject.toml` | ||
- it is declared under `tool.poetry.packages` - see other community modules | ||
- it is declared under `tool.poetry.extras` with the same name as your module name, | ||
we still prefer adding _NO EXTRA DEPENDENCIES_, meaning `mymodule = []` is the preferred addition | ||
(see the notes at the bottom) | ||
- [ ] The `INDEX.rst` at the project root includes your module under the `.. toctree` directive | ||
- [ ] Your branch is up to date (or we'll use GH's "update branch" function through the UI) | ||
|
||
# Preferred implementation | ||
|
||
- The current consensus among maintainers is to try to avoid enforcing the client library | ||
for the given tools you are triyng to implement. | ||
- This means we want you to avoid adding specific libraries as dependencies to `testcontainers`. | ||
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. type: trying |
||
- Therefore, you should implement the configuration and the waiting with as little extra as possible | ||
- You may still find it useful to add your preferred client library as a dev dependency |
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -1,13 +1,16 @@ | ||
ARG version=3.8 | ||
FROM python:${version} | ||
ARG PYTHON_VERSION | ||
FROM python:${version}-slim-bookworm | ||
|
||
WORKDIR /workspace | ||
RUN pip install --upgrade pip \ | ||
&& apt-get update \ | ||
&& apt-get install -y \ | ||
freetds-dev \ | ||
&& rm -rf /var/lib/apt/lists/* | ||
|
||
# install requirements we exported from poetry | ||
COPY build/requirements.txt requirements.txt | ||
COPY setup.py README.rst ./ | ||
RUN pip install -r requirements.txt | ||
|
||
# copy project source | ||
COPY . . |
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -1,84 +1,78 @@ | ||
PYTHON_VERSIONS = 3.9 3.10 3.11 | ||
.DEFAULT_GOAL := help | ||
|
||
|
||
PYTHON_VERSION ?= 3.10 | ||
IMAGE = testcontainers-python:${PYTHON_VERSION} | ||
RUN = docker run --rm -it | ||
# Get all directories that contain a setup.py and get the directory name. | ||
PACKAGES = core $(addprefix modules/,$(notdir $(wildcard modules/*))) | ||
|
||
# All */dist folders for each of the packages. | ||
DISTRIBUTIONS = $(addsuffix /dist,${PACKAGES}) | ||
UPLOAD = $(addsuffix /upload,${PACKAGES}) | ||
# All */tests folders for each of the test suites. | ||
TESTS = $(addsuffix /tests,$(filter-out meta,${PACKAGES})) | ||
TESTS_DIND = $(addsuffix -dind,${TESTS}) | ||
DOCTESTS = $(addsuffix /doctests,$(filter-out modules/README.md,${PACKAGES})) | ||
# All linting targets. | ||
LINT = $(addsuffix /lint,${PACKAGES}) | ||
|
||
# Targets to build a distribution for each package. | ||
dist: ${DISTRIBUTIONS} | ||
${DISTRIBUTIONS} : %/dist : %/setup.py | ||
cd $* \ | ||
&& python setup.py bdist_wheel \ | ||
&& twine check dist/* | ||
|
||
# Targets to run the test suite for each package. | ||
tests : ${TESTS} | ||
${TESTS} : %/tests : | ||
|
||
|
||
install: ## Set up the project for development | ||
poetry install --all-extras | ||
poetry run pre-commit install | ||
|
||
build: ## Build the python package | ||
poetry build && poetry run twine check dist/* | ||
|
||
tests: ${TESTS} ## Run tests for each package | ||
${TESTS}: %/tests: | ||
poetry run pytest -v --cov=testcontainers.$* $*/tests | ||
|
||
alexanderankin marked this conversation as resolved.
Show resolved
Hide resolved
|
||
# Target to combine and report coverage. | ||
coverage: | ||
coverage: ## Target to combine and report coverage. | ||
poetry run coverage combine | ||
poetry run coverage report | ||
poetry run coverage xml | ||
poetry run coverage html | ||
|
||
# Target to lint the code. | ||
lint: | ||
pre-commit run -a | ||
|
||
# Targets to publish packages. | ||
upload : ${UPLOAD} | ||
${UPLOAD} : %/upload : | ||
if [ ${TWINE_REPOSITORY}-$* = testpypi-meta ]; then \ | ||
echo "Cannot upload meta package to testpypi because of missing permissions."; \ | ||
else \ | ||
twine upload --non-interactive --skip-existing $*/dist/*; \ | ||
fi | ||
|
||
# Targets to build docker images | ||
image: | ||
lint: ## Lint all files in the project, which we also run in pre-commit | ||
poetry run pre-commit run -a | ||
|
||
image: ## Make the docker image for dind tests | ||
poetry export -f requirements.txt -o build/requirements.txt | ||
docker build --build-arg version=${PYTHON_VERSION} -t ${IMAGE} . | ||
docker build --build-arg PYTHON_VERSION=${PYTHON_VERSION} -t ${IMAGE} . | ||
|
||
# Targets to run tests in docker containers | ||
tests-dind : ${TESTS_DIND} | ||
DOCKER_RUN = docker run --rm -v /var/run/docker.sock:/var/run/docker.sock | ||
|
||
${TESTS_DIND} : %/tests-dind : image | ||
${RUN} -v /var/run/docker.sock:/var/run/docker.sock ${IMAGE} \ | ||
bash -c "make $*/lint $*/tests" | ||
tests-dind: ${TESTS_DIND} ## Run the tests in docker containers to test `dind` | ||
${TESTS_DIND}: %/tests-dind: image | ||
${DOCKER_RUN} ${IMAGE} \ | ||
bash -c "make $*/tests" | ||
|
||
# Target to build the documentation | ||
docs : | ||
docs: ## Build the docs for the project | ||
poetry run sphinx-build -nW . docs/_build | ||
|
||
# Target to build docs watching for changes as per https://stackoverflow.com/a/21389615 | ||
docs-watch : | ||
poetry run sphinx-autobuild . docs/_build # requires 'pip install sphinx-autobuild' | ||
|
||
doctests : ${DOCTESTS} | ||
doctests: ${DOCTESTS} ## Run doctests found across the documentation. | ||
poetry run sphinx-build -b doctest . docs/_build | ||
|
||
${DOCTESTS} : %/doctests : | ||
${DOCTESTS}: %/doctests: ## Run doctests found for a module. | ||
poetry run sphinx-build -b doctest -c doctests $* docs/_build | ||
|
||
# Remove any generated files. | ||
clean : | ||
|
||
clean: ## Remove generated files. | ||
rm -rf docs/_build | ||
rm -rf */build | ||
rm -rf */dist | ||
rm -rf build | ||
rm -rf dist | ||
rm -rf */*.egg-info | ||
|
||
clean-all: clean ## Remove all generated files and reset the local virtual environment | ||
rm -rf .venv | ||
|
||
# Targets that do not generate file-level artifacts. | ||
.PHONY : clean dists ${DISTRIBUTIONS} docs doctests image tests ${TESTS} | ||
.PHONY: clean docs doctests image tests ${TESTS} | ||
|
||
|
||
# Implements this pattern for autodocumenting Makefiles: | ||
# https://marmelab.com/blog/2016/02/29/auto-documented-makefile.html | ||
# | ||
alexanderankin marked this conversation as resolved.
Show resolved
Hide resolved
|
||
# Picks up all comments that start with a ## and are at the end of a target definition line. | ||
.PHONY: help | ||
help: ## Display command usage | ||
@grep -E '^[0-9a-zA-Z_-]+:.*?## .*$$' $(MAKEFILE_LIST) | sort | awk 'BEGIN {FS = ":.*?## "}; {printf "\033[36m%-30s\033[0m %s\n", $$1, $$2}' |
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Oops, something went wrong.
Oops, something went wrong.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
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.
I'm using the devcontainers workflow. I could add in a section on using that if you wanted.
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.
hey @bearrito apologies for the delays, I'm finding myself with some time today.
I'd love your contribution on the devcontainers workflow, but lemme finish this PR (ETA today, finally found the time to work on this) and then yo can raise your own PR for that addition to the docs 🙏