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

fix(kafka): wait_for_logs in kafka container to reduce lib requirement #377

Merged
merged 2 commits into from
Mar 24, 2024
Merged

fix(kafka): wait_for_logs in kafka container to reduce lib requirement #377

merged 2 commits into from
Mar 24, 2024

Conversation

gudjonragnar
Copy link
Contributor

Use wait_for_logs to wait for startup instead of waiting for successful connection via kafka-python. Also removes the dependency on kafka-python.

Closes #351

@gudjonragnar gudjonragnar requested a review from ash1425 as a code owner August 1, 2023 09:17
@jankatins
Copy link
Contributor

LGTM, we would really like to see this because we use confluent kafka in the project...

@jankatins
Copy link
Contributor

@gudjonragnar Will you find time to get this running on top of the current main branch? If not: would you mind if I take this and rebase (or rework, if needed) and open it as a new PR (I will keep your handle as co-author on the commit)?

@gudjonragnar
Copy link
Contributor Author

I will try to find some time beginning of next week 👍

@gudjonragnar
Copy link
Contributor Author

gudjonragnar commented Mar 3, 2024 •
edited
Loading

Should be up to date now, unless I missed something with the big changes done to the repo

jankatins
jankatins approved these changes Mar 3, 2024
View reviewed changes
Copy link
Contributor

@jankatins jankatins left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM (not a maintainer)

@alexanderankin
Copy link
Member

gudjonragnar#1

@totallyzen totallyzen changed the title Use wait_for_logs in kafka container fix(kafka): wait_for_logs in kafka container to reduce lib requirement Mar 4, 2024
@totallyzen totallyzen mentioned this pull request Mar 4, 2024
Closed
@gudjonragnar gudjonragnar force-pushed the kafka branch 2 times, most recently from a001530 to 77e2850 Compare March 4, 2024 10:52
@alexanderankin
Copy link
Member

Reminder to myself to get this merged #446

@alexanderankin alexanderankin merged commit 909107b into testcontainers:main Mar 24, 2024
9 checks passed
@github-actions github-actions bot mentioned this pull request Mar 24, 2024
Merged
bearrito pushed a commit to bearrito/testcontainers-python that referenced this pull request Mar 30, 2024
@gudjonragnar
fix(kafka): wait_for_logs in kafka container to reduce lib requirement (
0cc9a72 
testcontainers#377)

Use `wait_for_logs` to wait for startup instead of waiting for
successful connection via kafka-python. Also removes the dependency on
kafka-python.

Closes testcontainers#351

---------

Co-authored-by: Gudjon Ragnar Brynjarsson <[email protected]>
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/&lt;version&gt;`
([#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 -&gt; 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

@jankatins jankatins jankatins approved these changes

@ash1425 ash1425 Awaiting requested review from ash1425

Assignees
No one assigned
Labels
feat 👀 requires attention
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

Make the kafka package dependency optional to be able to use confluent_kafka
3 participants
@gudjonragnar @jankatins @alexanderankin