-
-
Notifications
You must be signed in to change notification settings - Fork 165
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
Add ChromaDB module #746
Add ChromaDB module #746
Conversation
✅ Deploy Preview for testcontainers-node ready!
To edit notification comments on pull requests, go to your Netlify site configuration. |
constructor(startedTestContainer: StartedTestContainer) { | ||
super(startedTestContainer); | ||
this.host = this.startedTestContainer.getHost(); | ||
this.port = this.startedTestContainer.getMappedPort(CHROMADB_PORT); |
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.
The port may change if the container is restarted, I would get the mapped port on demand each time instead of once on construction
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.
Thanks for the contribution! Could you please also add a page in the docs?
done! although it doesn't look like the netlify built again so I'm not 100% sure that I got the markdown right for the example. |
I just re run the jobs again |
Thanks, you'll need to add a link in the mkdocs.yml in the project root |
Okay, I've updated the readme to add a section about updating the docs, too in case that's useful :) |
Thanks, though that would be better placed in the CONTRIBUTING.md |
Okay, I've moved the bit about updating the docs into contributing and added a link to contributing.md from the readme |
ChromaDB already has testcontainers for Java and Go: https://testcontainers.com/modules/chroma/ This just adds it for Node as well.