chore(deps): Bumping Chroma version to 0.4.24 #2391
Conversation
✅ Deploy Preview for testcontainers-go ready!
To edit notification comments on pull requests, go to your Netlify site configuration. |
|
Hi @tazarov, thanks for submitting this PR! There is some context regarding the default versions that must be shared at some place from our side: we usually define a base image and let users use whatever image they need. In this particular case, the Chroma module does not add explicit logic for a given version, but there could be the case where we couple the implementation with a version (see Elasticsearch module as an example). Said that, I'd prefer updating only the examples and tests instead of the default image of the module. If this seems reasonable for you, I'll also update the example in the website here: https://github.com/testcontainers/community-module-registry/blob/4d037ebe011a53132bfca8ebd9f1984e13aa874b/modules/chroma/index.md?plain=1#L18 Thanks! |
|
@mdelapenya, thank you for the explanation. Let me ask this. Do you think it makes sense that default versions should be the ones that solve the most problems for the users they serve? The Chroma team made a lot of progress between 0.4.22 and 0.4.24. The team feels strongly about DX, having sensible defaults (e.g., bumping versions) so that the out-of-the-box experience feels part of that DX-first mindset as best as it could. That said, this is your project, and I have no objection to your comments. I am happy to close the PR or change the default to 0.4.22 in the |
|
@mdelapenya, another point in case: DONE 6 runs, 10 tests, 6 failures in 503.328s # 0.4.22
DONE 6 runs, 10 tests, 6 failures in 19.720s # 0.4.24
|
There was a problem hiding this comment.
@tazarov thanks for coming with such detailed information about the performance of the different versions.
I have to admit we don't have a strong opinion about the default versions yet. In fact we have internal discussions around what to define as a baseline, so we consider it's fine to merge this one.
Regarding DX, if we start updating the base images for any module at the moment a contributor comes with a bump for the base image of a given module, we would be potentially changing the DX for our users too, as it could change the expectations on how the underlying technology could work.
In this particular case it's a patch version, and we trust the Chroma team is doing a great job following semver, but it could be the case we bump a module in a major version that changes a healthcheck, or a listening port, or the log entries for the wait strategies, or a location for config files. It would mean updating not only the version but changing the implementation of the module.
We advocate for always passing the testcontainers.WithImage option to the RunContainer function (you know, users usually copy&paste from the docs), so I'm going to update the module page here: https://testcontainers.com/modules/chroma/
In any case, thank you so much for your contribution! 👏
|
@mdelapenya, I'm happy to update the docs. I have another PR with a few more examples and + a If you're ok with that, I can have the updated docs + the new PR out today. |
You can send the examples in a follow-up. If they are a demonstration for using the module, please use the testable examples pattern if possible 🙏 For me it's always difficult to create a testable example or a test, so I believe you'll find the place that makes sense to locate them 😅 |
|
BTW, are you planing to add testcontainers-go to the chroma-go project? That would be fantastic! and I can help out if needed |
|
@mdelapenya, I recently encountered test containers (from the |
|
Closing as per #2391 (comment) Thank you! |
What does this PR do?
Bumping Chroma version to latest stable 0.4.24
Why is it important?
The latest version introduces a number of bug fixes and improvements.
How to test this PR
Existing Chroma tests execute successfully.
--
I recently found out about your project and I think it is awesome. Keep up the good work