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.

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

refactor!: get rid of associated type ImageArgs and rename to cmd #649

Merged
merged 5 commits into from
Jun 10, 2024

Conversation

DDtKey
Copy link
Collaborator

@DDtKey DDtKey commented Jun 10, 2024

  • really common pattern is to use Vec<String>
  • args isn't the best term, because in fact it's Docker's CMD
  • it was not aligned with existing ExecCommand
  • current interface is confusing and users struggle with it
  • essentially it should be closer to the current representation of environment variables and other parameters
  • new implementation allows to override args regardless of underlying type
  • it's still will be possible to use custom strict arguments type for particular image which can be converted to list of strings if users want

Dependent PR to modules: testcontainers/testcontainers-rs-modules-community#143

Also, I'm preparing 2 follow-up PRs which will improve the user experience.

- really common pattern is to use `Vec<String>`
- `args` isn't the best term, because in fact it's Docker's [CMD](https://docs.docker.com/reference/dockerfile/#cmd)
- it was not aligned with existing `ExecCommand`
- current interface is confusing and users struggle with it
- essentially it should be closer to the current representation of environment variables and other parameters
- new implementation allows to override args regardless of underlying type
- it's still will be possible to use custom strict arguments type for particular image which can be converted to list of strings if users want

Dependent PR to modules: testcontainers/testcontainers-rs-modules-community#143
Copy link

netlify bot commented Jun 10, 2024

Deploy Preview for testcontainers-rust ready!

Name Link
🔨 Latest commit 7b0d455
🔍 Latest deploy log https://app.netlify.com/sites/testcontainers-rust/deploys/666770615e2a840008b7dafc
😎 Deploy Preview https://deploy-preview-649--testcontainers-rust.netlify.app
📱 Preview on mobile
Toggle QR Code...

QR Code

Use your smartphone camera to open QR code link.

To edit notification comments on pull requests, go to your Netlify site configuration.

DDtKey added a commit that referenced this pull request Jun 10, 2024
It's really helpful and allows some further refactoring for better API.

In most of the cases, it should not be a problem (and even pretty
common) to run test against fresher versions of Rust.

PR is needed for
#649 and couple
of follow-up PRs
@DDtKey DDtKey changed the title refactor!: get rid of associated type for ImageArgs refactor!: get rid of associated type for ImageArgs and rename to cmd Jun 10, 2024
@DDtKey DDtKey changed the title refactor!: get rid of associated type for ImageArgs and rename to cmd refactor!: get rid of associated type ImageArgs and rename to cmd Jun 10, 2024
@DDtKey DDtKey merged commit 2d41bdc into main Jun 10, 2024
13 checks passed
@DDtKey DDtKey deleted the refactor/arguments branch June 10, 2024 22:45
This was referenced Jun 15, 2024
DDtKey pushed a commit that referenced this pull request Jun 15, 2024
## 🤖 New release
* `testcontainers`: 0.17.0 -> 0.18.0

<details><summary><i><b>Changelog</b></i></summary><p>

<blockquote>

## [0.18.0] - 2024-06-15

### Details
#### Bug Fixes
- [❗] Make `DOCKER_CONFIG` usage consistent with Docker CLI
([#654](#654))

#### Features
- [❗] Support UDP and SCTP port mappings
([#655](#655))
- Impl `From<u16>` for `ContainerPort` with TCP default
([#658](#658))
- Support HTTP wait strategy
([#659](#659))
- Allow passing `u16` to `Ports`

#### Miscellaneous Tasks
- Use nightly `rustfmt`
([#657](#657))

#### Refactor
- [❗] Get rid of associated type `ImageArgs` and rename to `cmd`
([#649](#649))
- Avoid unnecessary owned structs and boxing
([#651](#651))
- [❗] Add `ImageExt` trait to avoid explicit conversion to
`RunnableImage`
([#652](#652))
- [❗] Rename `RunnableImage` to `ContainerRequest`
([#653](#653))
- [❗] Exposed and mapped ports api
([#656](#656))
- Preliminary refactoring of `wait` strategies
([#661](#661))
</blockquote>


</p></details>

---
This PR was generated with
[release-plz](https://github.com/MarcoIeni/release-plz/).

## Migration Guide

### Overview
Most of the breaking changes introduced in this version primarily impact
developers who implement their own images. For general usage, the API
remains mostly the same, with some improvements and enhancements for
better performance and flexibility.

### 1. Renaming of `RunnableImage`
- **Old**: `RunnableImage`
- **New**: `ContainerRequest`
- **Update**: The explicit conversion from `Image` to `ContainerRequest`
(formerly `RunnableImage`) is no longer necessary. Instead, you can now
directly import `testcontainers::ImageExt` and override image parameters
as needed.

### 2. Changes to `Image` Methods
- **Method**: `Image::tag` and `Image::name`
  - **Old Return Type**: `String`
  - **New Return Type**: `&str`
- **Update**: Update any code that relies on these methods to handle the
new return type `&str`. This change helps improve performance by
avoiding unnecessary allocations.

### 3. Changes to `Image::exposed_ports`
- **Method**: `Image::exposed_ports`
- **Old Return Type**: Implementation-specific or previously different.
  - **New Return Type**: `&[ContainerPort]`
- **Update**: The method now returns a slice of `ContainerPort`, which
supports exposing ports with protocols `TCP`, `UDP`, and `SCTP`. Update
your code to handle the slice accordingly.

### 4. Removal of Associated Type `Args` in `Image`
- **Old**: `Image` had an associated type `Args`.
- **New**: The associated type `Args` is removed.
- **Update**: Command arguments are now part of `Image::cmd`, which
returns `impl IntoIterator<Item = impl Into<String>>`. This change
allows more flexibility in specifying command arguments. Ensure your
code is updated to work with the new method signature.

### 5. Simplification of Trait Implementation in `Image`
- **Old**: `Image` required `Box<dyn ..>` for certain traits.
- **New**: Utilizes Return Position `impl` Trait in Trait (RPITIT).
- **Update**: Instead of requiring `Box<dyn ..>`, `Image` now uses
RPITIT for trait returns. This change simplifies the code and improves
readability and performance. Familiarize yourself with
[RPITIT](https://rustc-dev-guide.rust-lang.org/return-position-impl-trait-in-trait.html)
to understand its benefits and applications in your implementation.

### 6. Changes to `RunnableImage::with_mapped_port`
- **Old**: `RunnableImage::with_mapped_port`
- **New**: Accessible through `ImageExt::with_mapped_port`
- **Update**: This method now accepts two parameters instead of a tuple.
Adjust your method calls to pass the parameters separately.

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
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

1 participant