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

fix(core): make config editable to avoid monkeypatching.1 #532

Merged
merged 7 commits into from
Apr 8, 2024

Conversation

alexanderankin
Copy link
Collaborator

see #531

@alexanderankin
Copy link
Collaborator Author

ok, i checked against #519 and shouldnt be too hard to rebase (if even necessary), unfortunately re #525 and #527 - 527 is a draft and 525 is a feat, which deprioritizes over things which can be considered fixes - unless the changes are inspired by tc-go or tc-java or other sources of truth on how to deal with things like mssql, may even be "breaking" even though they are "fixing" - i unfortunately have a lot of pent up sympathy for people doing unusual software things. nothing against the PR, we will figure it out and hopefully release it as a fix rather than a feat)

so with that said, not sure why i shouldnt merge this

@alexanderankin alexanderankin changed the title fix(core): make config editable so users don't have to monkey patch the library fix(core): make config editable to avoid monkeypatching.1 Apr 8, 2024
@alexanderankin alexanderankin merged commit 3be6da3 into main Apr 8, 2024
29 checks passed
@alexanderankin alexanderankin deleted the editable_config branch April 8, 2024 09:24
alexanderankin pushed a commit that referenced this pull request Apr 8, 2024
🤖 I have created a release *beep* *boop*
---


##
[4.3.2](testcontainers-v4.3.1...testcontainers-v4.3.2)
(2024-04-08)


### Bug Fixes

* **core:** Improve typing for common container usage scenarios
([#523](#523))
([d5b8553](d5b8553))
* **core:** make config editable to avoid monkeypatching.1
([#532](#532))
([3be6da3](3be6da3))
* **vault:** add support for HashiCorp Vault container
([#366](#366))
([1326278](1326278))

---
This PR was generated with [Release
Please](https://github.com/googleapis/release-please). See
[documentation](https://github.com/googleapis/release-please#release-please).

Co-authored-by: github-actions[bot] <41898282+github-actions[bot]@users.noreply.github.com>
mloesch pushed a commit to mloesch/testcontainers-python that referenced this pull request Apr 8, 2024
…ners#532)

see testcontainers#531:

I am using testcontainers within a library that provides some
pytest-fixtures.
In order for this to work I have change some settings.

As I can not guarantee that that my lib is imported before
testcontainers I need to monkeypatch the settings.
This is much easier if I only need to monkeypatch the config file and
not all modules that use configurations.

I would argue that for a potential library as this, this is a better
design.

Also one can easier see that the given UPERCASE variable is not a
constant but rather a setting.

Co-authored-by: Carli* Freudenberg <carli.freudenberg@energymeteo.de>
mloesch pushed a commit to mloesch/testcontainers-python that referenced this pull request Apr 8, 2024
🤖 I have created a release *beep* *boop*
---


##
[4.3.2](testcontainers/testcontainers-python@testcontainers-v4.3.1...testcontainers-v4.3.2)
(2024-04-08)


### Bug Fixes

* **core:** Improve typing for common container usage scenarios
([testcontainers#523](testcontainers#523))
([d5b8553](testcontainers@d5b8553))
* **core:** make config editable to avoid monkeypatching.1
([testcontainers#532](testcontainers#532))
([3be6da3](testcontainers@3be6da3))
* **vault:** add support for HashiCorp Vault container
([testcontainers#366](testcontainers#366))
([1326278](testcontainers@1326278))

---
This PR was generated with [Release
Please](https://github.com/googleapis/release-please). See
[documentation](https://github.com/googleapis/release-please#release-please).

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

CarliJoy commented Apr 8, 2024

Dear, @alexanderankin

Imports like

from testcontainers.core.config import testcontainers_config as config

will have the same problems as

from testcontainers.core.config import SETTING

Only difference is that one can monkeypatch values of the testcontainers_config dataclass object.

But if you goal was to allow to make testcontainers_config replaceable with a new TestcontainersConfiguration instance, it won't work.

But thank you for adopting my idea so fast.
It's good enough for me.

@alexanderankin
Copy link
Collaborator Author

alexanderankin commented Apr 8, 2024 via email

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.

None yet

2 participants