-
Notifications
You must be signed in to change notification settings - Fork 0
valkey test container #1
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
base: main
Are you sure you want to change the base?
Conversation
| """ | ||
| return int(super().get_exposed_port(self.port)) | ||
|
|
||
| @wait_container_is_ready(ValkeyNotReady) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Deprecated Decorator Usage
Location: modules/valkey/testcontainers/valkey/__init__.py, line 113
Current Code:
@wait_container_is_ready(ValkeyNotReady)
def _connect(self) -> None:
"""Wait for Valkey to be ready by sending PING command."""
# ... connection logicIssue:
The @wait_container_is_ready decorator is deprecated in the codebase. Recent commits show active migration away from this pattern:
- PR feat: Add ExecWaitStrategy and migrate Postgres from deprecated decorator testcontainers/testcontainers-python#935: Postgres migrated to
ExecWaitStrategy - PR fix(minio): Use wait strategy instead of deprecated decorator testcontainers/testcontainers-python#899: Minio migrated to wait strategies
- PR fix(elasticsearch): Use wait strategy instead of deprecated decorator testcontainers/testcontainers-python#915: Elasticsearch migrated to wait strategies
Evidence from Codebase:
# From pytest.ini_options filterwarnings:
"ignore:The @wait_container_is_ready decorator is deprecated.*:DeprecationWarning"Recommended Fix:
Migrate to modern wait strategy pattern:
from testcontainers.core.wait_strategies import ExecWaitStrategy
class ValkeyContainer(DockerContainer):
def __init__(self, image: str = "valkey/valkey:latest", port: int = 6379, **kwargs) -> None:
super().__init__(image, **kwargs)
self.port = port
self.password: Optional[str] = None
self.with_exposed_ports(self.port)
def start(self) -> "ValkeyContainer":
# Build wait strategy based on password
if self.password:
# Use custom wait strategy for authenticated connections
self.waiting_for(self._create_auth_wait_strategy())
else:
# Use exec strategy for simple PING
self.waiting_for(
ExecWaitStrategy(["valkey-cli", "ping"])
)
super().start()
return selfBenefits:
- Aligns with project direction
- More composable and testable
- Consistent with other modules
- Better error messages
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
fixed
No description provided.