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.

Sign up for GitHub

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

Jump to bottom

feat: allow configuring resource ulimits #714

Merged
merged 7 commits into from
Aug 3, 2024
Merged

feat: allow configuring resource ulimits #714

merged 7 commits into from
Aug 3, 2024

Conversation

blaenk
Copy link
Contributor

@blaenk blaenk commented Aug 2, 2024 •
edited
Loading

Thanks again for working on this project!

This exposes a way to configure resource ulimits (the --ulimit flag for docker run).

I'm relying on CI tests because I'm currently on Windows and didn't configure my environment to reflect #711, since it seems I'd need to install nasm now, as I received some errors regarding it.

@netlify Netlify
Copy link

netlify bot commented Aug 2, 2024 •
edited
Loading

✅ Deploy Preview for testcontainers-rust ready!

Name Link
🔨 Latest commit 8540392
🔍 Latest deploy log https://app.netlify.com/sites/testcontainers-rust/deploys/66aea0ae40f2470008204c47
😎 Deploy Preview https://deploy-preview-714--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.

@blaenk blaenk changed the title Allow configuring resource ulimits feat: allow configuring resource ulimits Aug 2, 2024
@DDtKey
Copy link
Collaborator

DDtKey commented Aug 2, 2024

I'm relying on CI tests because I'm currently on Windows and didn't configure my environment to reflect #711, since it seems I'd need to install nasm now, as I received some errors regarding it.

Yeah, unfortunately this requirement comes from rustls with aws-lc-rs driver.

But bollard merged the fix not to require the driver except ring:
fussybeaver/bollard#441

So, as workaround I may suggest using patch of bollard in your Cargo.toml to git one.
However I can't guarantee there is no other deps in your lock file which keeps rustls default features enabled.

DDtKey
DDtKey reviewed Aug 2, 2024
View reviewed changes
Copy link
Collaborator

@DDtKey DDtKey left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thank you for the contribution! 🙏
I've left only one comment, please take a look. Overall looks like LGTM

testcontainers/src/core/containers/request.rs Outdated Show resolved Hide resolved
@DDtKey DDtKey merged commit b10b48f into testcontainers:main Aug 3, 2024
12 checks passed
@blaenk
Copy link
Contributor Author

blaenk commented Aug 3, 2024

Thank you for the review!

No rush at all, but I'm curious when you can imagine a release including it.

@DDtKey
Copy link
Collaborator

DDtKey commented Aug 4, 2024 •
edited
Loading

I think within couple of days since it’s compatible change.
I just need to check if there is anything else I wanted to finish before proceeding with the new release

@github-actions github-actions bot mentioned this pull request Aug 5, 2024
Merged
DDtKey pushed a commit that referenced this pull request Aug 5, 2024
@github-actions
chore: release v0.21.1 (#717)
3f30ac8 
## 🤖 New release
* `testcontainers`: 0.21.0 -> 0.21.1

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

<blockquote>

## [0.21.1] - 2024-08-05

### Details
#### Features
- Allow configuring resource ulimits
([#714](#714))
</blockquote>


</p></details>

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

Co-authored-by: github-actions[bot] <41898282+github-actions[bot]@users.noreply.github.com>
@DDtKey
Copy link
Collaborator

DDtKey commented Aug 5, 2024

It's been released!

@blaenk
Copy link
Contributor Author

blaenk commented Aug 5, 2024

I appreciate the heads up thank you!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@DDtKey DDtKey DDtKey approved these changes

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
2 participants
@blaenk @DDtKey