Skip to content
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.

Sign up for GitHub

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

Jump to bottom

feat(new): add a new Docker Registry test container #389

Merged
merged 5 commits into from
Mar 31, 2024
Merged

feat(new): add a new Docker Registry test container #389

merged 5 commits into from
Mar 31, 2024

Conversation

max-pfeiffer
Copy link
Contributor

I added a new test container for spinning up a Docker registry.

@max-pfeiffer max-pfeiffer force-pushed the feature/add_docker_registry branch from 571d474 to 85f7451 Compare October 29, 2023 19:13
@max-pfeiffer max-pfeiffer force-pushed the feature/add_docker_registry branch 4 times, most recently from 27601db to 12cbe3e Compare November 29, 2023 21:55
@max-pfeiffer max-pfeiffer mentioned this pull request Dec 2, 2023
Closed
@max-pfeiffer max-pfeiffer force-pushed the feature/add_docker_registry branch from 12cbe3e to 98d1245 Compare December 6, 2023 18:24
@totallyzen
Copy link
Collaborator

Hi @max-pfeiffer I'm grooming the ongoing issues for the project (we're rebooting the maintenance, see #405) and I'd like to know more about your test case!

It's nice to have many flavours of containers, but it also means exponentially increasing complexity and cost of running the tests for this project! Therefore we're trying to stick to the most commonly used containers for now.

@alexanderankin
Copy link
Member

Since 2024 I think the case may (!) actually be stronger than ever to support it(??)

@max-pfeiffer
Copy link
Contributor Author

Hi @totallyzen, when building Docker containers with Docker Buildx there is currently no option to test your containers locally without a local registry. Otherwise Buildx pushes your image to Docker Hub, which is not what you want in a test case. More and more you need to use Buildx for effienciently building images and especially multi arch images.

When you use Docker Python libraries like docker-py or python-on-whales to build and test Docker images, what a lot of persons and DevOps engineers like me nowadays do, a test container comes in very handy.

BTW: I use the testcontainers package quite a bit and have time to spare. If you still need a helping hand I would be happy to jump in.

@totallyzen
Copy link
Collaborator

totallyzen commented Mar 1, 2024 •
edited
Loading

@max-pfeiffer thanks, exactly what I meant to express! 😄 Not to push back, but to explain why this is useful (and put it in the readme of the module) 🙏

@alexanderankin indeed, I meant more the "average developer" use case rather than the specialised use case of the Docker company testing/building their own software! :) If it's only the latter, there would have been arguments that they have been successful without AND they develop their software in Go these days.

Having said that,

Max you're very welcome to join! You can start by rebasing your own PR to match the new project structure:

  • put your source-code in modules/* - basically copy/paste except NO setup.py
  • you put your package in pyproject.toml in here
  • you need to declare an extra even if you don't have any current deps (for future-proofing and making CI code simpler to write) like nginx for example
  • instead of main.yml you put your module in ci-community.yml

After that happy to review your PR and see what you'd like to work on next! 🙏

@max-pfeiffer
Copy link
Contributor Author

@totallyzen Thanks for the info. I will work on it this weekend.

BTW: I just stumbled across that page: https://github.com/testcontainers/testcontainers-python/contribute
It seemed to be outdated as you guys just refactor the project.

@totallyzen
Copy link
Collaborator

It seemed to be outdated as you guys just refactor the project.

Yes, very much, sorry about that. We're both operating on spare time and energy.
Fixes to the docs are highly appreciated! ❤️

@max-pfeiffer max-pfeiffer force-pushed the feature/add_docker_registry branch from 98d1245 to 0278346 Compare March 2, 2024 12:23
@max-pfeiffer
Copy link
Contributor Author

max-pfeiffer commented Mar 2, 2024 •
edited
Loading

@totallyzen I just rebased my PR with main branch and did the refactoring to match the new project setup.

Nice work, I like that setup with Poetry. Also I think it's good to switch to Ruff for code formatting and linting. Ruff is much more performant and easier to configure as all the other common tools.

I struggle to understand what I need to do in ci-community.yml. Looking at that workflow there seems to be nothing to add for me as module changes are tracked automatically.

@totallyzen totallyzen changed the title Added a new Docker registry test container feat(new): add a new Docker Registry test container Mar 2, 2024
@totallyzen
Copy link
Collaborator

hey @max-pfeiffer indeed, ci-community.yml is auto-tracking modules as long as you add them in the pyproject.toml, which you did!

I ran the checks for you, seems like some docs and linting errors were made.
I think you should be able to see the output and fix them!
I can also highly recommend installing pre-commit and running it so you get early feedback! :)

Other than that, you're almost done here! Thanks for the effort!

@max-pfeiffer
Copy link
Contributor Author

max-pfeiffer commented Mar 2, 2024 •
edited
Loading

Hi @totallyzen , hmm I saw that that pre-commit config in project root, but pyproject.toml was missing the pre-commit dependencies (pre-commit, black, ruff). So I would say the pre-commit setup is not really there and ready to use for software engineers. 😃

I quickly fixed that in this PR: #438
Is this something you think we could merge in for completing this PR?

@totallyzen totallyzen mentioned this pull request Mar 7, 2024
Open
@alexanderankin alexanderankin added ✨ package: new community-feat feature but its a community module so we wont bump tc core for it and removed feat labels Mar 31, 2024
@alexanderankin alexanderankin force-pushed the feature/add_docker_registry branch from a750f8f to 13ac882 Compare March 31, 2024 03:13
@alexanderankin alexanderankin merged commit 0f554fb into testcontainers:main Mar 31, 2024
12 checks passed
@github-actions github-actions bot mentioned this pull request Mar 31, 2024
Merged
alexanderankin pushed a commit that referenced this pull request Apr 1, 2024
@github-actions
chore(main): release testcontainers 4.3.0 (#500)
f819c7a 
🤖 I have created a release *beep* *boop*
---


##
[4.3.0](testcontainers-v4.2.0...testcontainers-v4.3.0)
(2024-04-01)


### Features

* **client:** Add custom User-Agent in Docker client as
`tc-python/<version>`
([#507](#507))
([dd55082](dd55082))


### Bug Fixes

* Add CassandraContainer
([#476](#476))
([507e466](507e466))
* add chroma container
([#515](#515))
([0729bf4](0729bf4))
* Add Weaviate module
([#492](#492))
([90762e8](90762e8))
* **cassandra:** make cassandra dependency optional/test-only
([#518](#518))
([bddbaeb](bddbaeb))
* **core:** allow setting docker command path for docker compose
([#512](#512))
([63fcd52](63fcd52))
* **google:** add support for Datastore emulator
([#508](#508))
([3d891a5](3d891a5))
* Improved Oracle DB module
([#363](#363))
([6e6d8e3](6e6d8e3))
* inconsistent test runs for community modules
([#497](#497))
([914f1e5](914f1e5))
* **kafka:** Add redpanda testcontainer module
([#441](#441))
([451d278](451d278))
* **kafka:** wait_for_logs in kafka container to reduce lib requirement
([#377](#377))
([909107b](909107b))
* **keycloak:** container should use dedicated API endpoints to
determine container readiness
([#490](#490))
([2e27225](2e27225))
* **nats:** Client-Free(ish) NATS container
([#462](#462))
([302c73d](302c73d))
* **new:** add a new Docker Registry test container
([#389](#389))
([0f554fb](0f554fb))
* pass doctests, s/doctest/doctests/, run them in gha,
s/asyncpg/psycopg/ in doctest, fix keycloak flakiness: wait for first
user
([#505](#505))
([545240d](545240d))
* pass updated keyword args to Publisher/Subscriber client in
google/pubsub
[#161](#161)
([#164](#164))
([8addc11](8addc11))
* Qdrant module
([#463](#463))
([e8876f4](e8876f4))
* remove accidentally added pip in dev dependencies
([#516](#516))
([dee20a7](dee20a7))
* **ryuk:** Enable Ryuk test suite. Ryuk image 0.5.1 -> 0.7.0. Add
RYUK_RECONNECTION_TIMEOUT env variable
([#509](#509))
([472b2c2](472b2c2))

---
This PR was generated with [Release
Please](https://github.com/googleapis/release-please). See
[documentation](https://github.com/googleapis/release-please#release-please).

Co-authored-by: github-actions[bot] <41898282+github-actions[bot]@users.noreply.github.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers
No reviews
Assignees
No one assigned
Labels
community-feat feature but its a community module so we wont bump tc core for it ✨ package: new
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
3 participants
@max-pfeiffer @totallyzen @alexanderankin