8000 Remove class-level variables from containers. by tillahoffmann · Pull Request #296 · testcontainers/testcontainers-python · GitHub
[go: up one dir, main page]
Skip to content

Remove class-level variables from containers. #296

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
Merged
tillahoffmann merged 34 commits into from
Apr 11, 2023
Merged

Remove class-level variables from containers. #296

tillahoffmann merged 34 commits into from
Apr 11, 2023

Conversation

@tillahoffmann
Copy link
Contributor
@tillahoffmann tillahoffmann commented Jan 7, 2023 •
edited
Loading

Several of the containers currently load environment variables at import time. Consequently, if we create a container, its configuration may not match the current environment variables. Containers also overwrite class variables with instance variables of the same name which can cause confusion.

This PR

  • removes all class-level variables and loads environment variables at the time the container is created.
  • unifies the container API by renaming user to username and port_to_expose to port where applicable such that all containers have the same parameter names. The motivation behind renaming port_to_expose is that some containers have multiple different ports (cf. Clickhouse container exposing ports setup is strange #257 and Add azurite module #234). [specific]_port_to_expose is just a bit long.

This will break a bunch of stuff, but I'm planning to squeeze a lot of this cleanup into the next major release.

@tillahoffmann tillahoffmann mentioned this pull request Jan 10, 2023
Closed
7 tasks
This was referenced Feb 17, 2023
Closed
Open
@tillahoffmann
Remove class-level variables from azurite.
5f5536a
@tillahoffmann
Precompute TIMEOUT.
7a06617
@tillahoffmann
Remove static variables from clickhouse.
ad3aef7
@tillahoffmann
Remove static variables from kafka.
355ac71
@tillahoffmann
Remove static variables from keycloak.
d357376
@tillahoffmann
Remove static variables from mongodb.
bfb8200
@tillahoffmann
Remove static variables from mssql.
e072dba
@tillahoffmann
Remove static variables from mysql.
1d6e402
@tillahoffmann
Remove static variables from neo4j.
e8e4078
@tillahoffmann
Rename user to username in opensearch.
043dea0
@tillahoffmann
Remove static variables from postgres.
d407859
@tillahoffmann
Remove static variables from rabbitmq.
23e93b2
@tillahoffmann
Use f-strings throughout.
33014e4
@tillahoffmann
Fix typo in ClickHouseContainer.__init__.
95dc6ca
@tillahoffmann
Fix default username in neo4j.
14761b1
@tillahoffmann
Rename port_to_expose to port and use f-strings in tests.
cf6c585
@tillahoffmann
Remove erroneous with_command statement.
3c28cdd
@tillahoffmann
Rename db_user to username in tests.
89822f7
@tillahoffmann
Remove unused function in docker api test.
deebd05
@tillahoffmann
Rename db_name to dbname.
44d9b3d
@tillahoffmann
Remove uninformative module docstrings.
7f08e44
@tillahoffmann
Rename port_to_expose to port for Redis and Elasticsearch.
f5a16cb
@tillahoffmann
Use utility function to raise for deprecated parameters.
3add8cb
pffijt
pffijt reviewed Feb 23, 2023
View reviewed changes
@pffijt pffijt self-requested a review February 23, 2023 11:36
@pffijt
Copy link
Contributor
pffijt commented Feb 23, 2023 •
edited
Loading

We should fix the minor inconsistencies then this pr should be ready for merge.

@tillahoffmann tillahoffmann requested a review from nshine as a code owner April 11, 2023 14:14
@tillahoffmann tillahoffmann merged commit 63ad996 into testcontainers:master Apr 11, 2023
@tillahoffmann tillahoffmann deleted the pythonic branch April 11, 2023 15:42
@tillahoffmann tillahoffmann mentioned this pull request Apr 11, 2023
Open
@tillahoffmann tillahoffmann mentioned this pull request May 19, 2023
Open
@shunping shunping mentioned this pull request Jul 1, 2025
Merged
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@maltehedderich maltehedderich Awaiting requested review from maltehedderich

@timbmg timbmg Awaiting requested review from timbmg

@ash1425 ash1425 Awaiting requested review from ash1425

@yakimka yakimka Awaiting requested review from yakimka

@nshine nshine Awaiting requested review from nshine

2 more reviewers

@ImFlog ImFlog ImFlog approved these changes

@pffijt pffijt pffijt approved these changes

Reviewers whose approvals may not affect merge requirements

Assignees

No one assigned

Labels

🔧 maintenance

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

3 participants

@tillahoffmann @pffijt @ImFlog
0