feat(nats): Add new module #439
Conversation
|
@totallyzen Is this good to go? |
|
Hey @bearrito can you give a bit more context on the use-case this covers? Cons:
Alternatives:
WDYT? |
|
I'm not quite sure I understand the comments. This adds a new module for the Nats streaming pub/sub project. Is the suggestion that instead of creating a new module for every container that we encapsulate that into docker compose files? I can see how that would simplify some things If that's the case how do we add convenience methods for users without adding additional libraries to poetry? |
|
i am also confused by the lot to unpack and the compose suggestion. certainly you will be up and running faster with a compose file or adding this class to your project's test code than waiting for us because you can do that right away, but i dont think that means we are not adding any future modules
this is the real pickle - we basically want to avoid adding other libraries public apis to our public api i would actually offer different feedback which is that we should probably use a log waiting strategy - testcontainers/testcontainers-java#1733 |
|
@alexanderankin Yeah, I can add the log waiting strategy in. No issue, if that's the only blocker I can do that immediately. |
|
I tried to write down my priorities in one of these tickets but lost it, a
key one is pairing down dependencies - some folks unhappy with postgres
something or other - but generally waiting for logs seems to make more
people happier
…On Wed, Mar 6, 2024, 1:47 PM Barrett Strausser ***@***.***> wrote:
@alexanderankin <https://github.com/alexanderankin> Yeah, I can add the
log waiting strategy in. No issue, if that's the only blocker I can do that
immediately.
However, I don't want to add it if, the suggestion is : Do the this whole
other thing with compose files.
—
Reply to this email directly, view it on GitHub
<#439 (comment)>,
or unsubscribe
<https://github.com/notifications/unsubscribe-auth/ACECGJCSQCBNKOHK26WWIXLYW5QD3AVCNFSM6AAAAABEDNK5HCVHI2DSMVQWIX3LMV43OSLTON2WKQ3PNVWWK3TUHMYTSOBRGU2TQNZXGA>
.
You are receiving this because you were mentioned.Message ID:
***@***.***>
|
|
@alexanderankin Updated with logs waiting. Side note it might be nice to expose the max wait time for the log wait. Right now it's set in the config file (which I could abuse). I think I'd generally want it less than 2 mins especially in CI where I could end up waiting NUM_TESTS * 2 mins. It does throw, but then it's considered a TRANSIENT and we have to wait. Either way.. |
|
Hey,
Been out for my evening. @alexanderankin is right, while I prefer using core
libraries, it's certainly a simpler solution if you can make it with as few
extra dependencies as possible.
Cheers!
…On Wed, 6 Mar 2024, 19:47 Barrett Strausser, ***@***.***> wrote:
@alexanderankin <https://github.com/alexanderankin> Yeah, I can add the
log waiting strategy in. No issue, if that's the only blocker I can do that
immediately.
However, I don't want to add it if, the suggestion is : Do the this whole
other thing with compose files.
—
Reply to this email directly, view it on GitHub
<#439 (comment)>,
or unsubscribe
<https://github.com/notifications/unsubscribe-auth/AJQBTD23S57PMMPXXDVCIE3YW5QD3AVCNFSM6AAAAABEDNK5HCVHI2DSMVQWIX3LMV43OSLTON2WKQ3PNVWWK3TUHMYTSOBRGU2TQNZXGA>
.
You are receiving this because you were mentioned.Message ID:
***@***.***>
|
Fair point! This could be added to the ton of other safeguarding improvements, particularly #314 where we'd introduce a time-limit to container existence (thus ensuring that you don't leak test containers) |
|
@totallyzen or @alexanderankin For this go around, since I've implemented the log based wait, is this good to go (after I fix the conflict)? Or do I need to go another direction? |
|
Let me take a look I haven't gotten a chance yet even at the old one. If it
looks good I will merge, I don't see why not. Mostly time bottlenecked on
my end.
…On Wed, Mar 6, 2024, 5:45 PM Barrett Strausser ***@***.***> wrote:
@totallyzen <https://github.com/totallyzen> or @alexanderankin
<https://github.com/alexanderankin> For this go around, since I've
implemented the log based wait, is this good to go (after I fix the
conflict)? Or do I need to go another direction?
—
Reply to this email directly, view it on GitHub
<#439 (comment)>,
or unsubscribe
<https://github.com/notifications/unsubscribe-auth/ACECGJBOUTQHQKWPFEHFO7TYW6MARAVCNFSM6AAAAABEDNK5HCVHI2DSMVQWIX3LMV43OSLTON2WKQ3PNVWWK3TUHMYTSOBRHE4DIMJRGY>
.
You are receiving this because you were mentioned.Message ID:
***@***.***>
|
…ntainers#454) - updated the image versions used for tests with the latest supported tags: https://hub.docker.com/_/elasticsearch - fixes testcontainers#452 
im not actually sure if this fixes it but does: make clickhouse waiting look more like java-tc many other discussions about removing dependency on sqlalchemy + drivers --------- Co-authored-by: Bálint Bartha <[email protected]>
…ainers#380) Fixes the following warning ``` sys:1: ResourceWarning: unclosed <socket.socket fd=7, family=AddressFamily.AF_UNIX, type=SocketKind.SOCK_STREAM, proto=0, raddr=/Users/rodrigo/.docker/run/docker.sock> ``` Related to testcontainers#379 Co-authored-by: David Ankin <[email protected]>
The current one references 2017, and it is confusing since the website may be perceived as outdated one --------- Co-authored-by: Till Hoffmann <[email protected]> Co-authored-by: David Ankin <[email protected]>
…ll before?) (testcontainers#461) we were using this code to test if it was online or not:`MongoClient(self.get_connection_url())`, but that doesn't actually perform any connection, instead you have to do something like: ```python @wait_container_is_ready() def _connect(self): client = self.get_connection_client() # will raise pymongo.errors.ServerSelectionTimeoutError if no connection is established client.admin.command('ismaster') ``` thanks to @smparekh for pointing this out, in his PR: https://github.com/testcontainers/testcontainers-python/pull/80/files#diff-cf09f76f44db0af04c58ddb456ccae39f7e29ce1d9208acd5f514c0a7dccb646R78 this PR implements the workaround described in the PR: ```python @pytest.fixture(scope="session") def test_client(): # init mongo mongo_container = MongoDbContainer("mongo:4").start() wait_for_logs(mongo_container, 'waiting for connections on port 27017') ... Co-authored-by: Shaishav Parekh <[email protected]>
|
there was no need to merge - there are no conflicts - see alexanderankin#6 |
Adds support for NATS. https://nats.io/
I wouldn't merge this yet. It's a bit awkward in determining if the container is ready (because of async) in the
startmethod.I'll sort out how I want to do that.