Conversation
✅ Deploy Preview for testcontainers-rust ready!
To edit notification comments on pull requests, go to your Netlify project configuration. |
| /// Represents a filesystem mount. | ||
| /// For more information see [Docker Storage](https://docs.docker.com/storage/) documentation. | ||
| #[derive(Debug, Clone)] | ||
| #[derive(Debug, Clone, PartialEq)] |
There was a problem hiding this comment.
That might not work as expected because equality here isn’t just about comparing all fields. For instance, if access, permissions or tmpfs size are different but point to the same target path, should it be considered equal or not? It depends on the use case - but in most of cases you most likely only interested in unique mount per target or (source, target)
There was a problem hiding this comment.
the way I implemented it now works for my use case (I'm using a fork of testconainers as a dependency to my project until this PR is merged).
If someone has a use case where they only care about some fields but not others, then a method can be added for those cases, like Mount::eq_target_path(other: Mount)
There was a problem hiding this comment.
Let me explain my point.
Quoting the original issue:
I want to check whether the container already has the mount before pushing the mount. This is to prevent duplicate mounts, which otherwise triggers error.
That reads to me like: the goal is to prevent mount conflicts, not just detect structurally identical Mount values.
#[derive(PartialEq)] only gives structural equality (all fields equal). It will dedup exact duplicates, but it won’t catch the common case where two mounts target the same container path with different options.
So this may “work” for now for exact duplicates, but it doesn’t address the broader dedup/conflict scenario described in the issue and could break once another use case appears.
Even in your case, let's say if you simply adjust a size for example.
I’d suggest either:
• adding an explicit same-target / conflict check, or
• keying mounts by target
Deriving PartialEq is fine for structural equality, but I wouldn’t rely on == as the semantic notion of "duplicate mount".
There was a problem hiding this comment.
What do you think about using a method then for doing what I did via PartialEq? Or do you have an idea for how to implement PartialEq in a better way?
2 participants
fixes #911