-
Couldn't load subscription status.
- Fork 6
CPBR-3041 replace docker compose with official docker sdk #205
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: master
Are you sure you want to change the base?
Conversation
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.
Pull Request Overview
This PR replaces the deprecated docker-compose Python library with the official Docker SDK, providing a modern implementation that maintains compatibility with Python 3.10+ while preserving the existing TestCluster interface.
- Implements a complete replacement for docker-compose functionality using Docker SDK
- Maintains backward compatibility with existing TestCluster interface
- Updates all Docker API calls to use the modern Docker SDK patterns
Reviewed Changes
Copilot reviewed 2 out of 3 changed files in this pull request and generated 4 comments.
| File | Description |
|---|---|
| confluent/docker_utils/compose_replacement.py | New module implementing docker-compose functionality using Docker SDK with classes for config parsing, container management, and project orchestration |
| confluent/docker_utils/init.py | Updates imports and refactors existing functions to use Docker SDK instead of deprecated compose library |
Tip: Customize your code reviews with copilot-instructions.md. Create the file or learn how to get started.
| def is_running(self) -> bool: | ||
| """Check if container is running.""" | ||
| self.container.reload() | ||
| return self.container.status == ContainerStatus.RUNNING.value |
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.
this can be made simpler by converting ContainerStatus to StrEnum. If only use is with .value then it makes sense to use StrEnum.
| STATUS = "Status" | ||
|
|
||
|
|
||
| # File I/O Constants |
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.
remove these redundant comment as it is clear from the variable name itself.
| # File I/O Constants | ||
| FILE_READ_MODE = "r" | ||
|
|
||
| # Docker Volume Constants |
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.
remove these redundant comment as it is clear from the variable name itself.
| WORKING_DIR = "working_dir" | ||
|
|
||
|
|
||
| class DockerStateKeys(StrEnum): |
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.
these don't seem to go well together, it might be better to use normal constants for this.
| from enum import Enum, StrEnum | ||
|
|
||
|
|
||
| # Container Status Enums |
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.
remove these redundant comment as it is clear from class Name itself.
| # In a full implementation, you'd handle building here | ||
| raise NotImplementedError("Building images not implemented in this example") | ||
|
|
||
| # Command |
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.
remove these redundant comment
| if ComposeConfigKeys.COMMAND in service_config: | ||
| config[ComposeConfigKeys.COMMAND] = service_config[ComposeConfigKeys.COMMAND] | ||
|
|
||
| # Environment variables |
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.
remove these redundant comment
| else: | ||
| config[ComposeConfigKeys.ENVIRONMENT] = env | ||
|
|
||
| # Ports |
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.
remove these redundant comment
| ports[str(port_mapping)] = None | ||
| config[ComposeConfigKeys.PORTS] = ports | ||
|
|
||
| # Volumes |
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.
remove these redundant comment
| volumes[host_path] = {VOLUME_BIND_MODE: container_path, 'mode': VOLUME_READ_WRITE_MODE} | ||
| config[ComposeConfigKeys.VOLUMES] = volumes | ||
|
|
||
| # Working directory |
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.
remove these redundant comment
Change Description
Replaced docker-compose with a utility based on docker official's sdk for python
Testing
Tested with existing tests under test_docker_utils.py
cub and dub do not use the docker_utils so no impact on cub/dub
tested docker utils by using them in base image and running integration tests: https://semaphore.ci.confluent.io/workflows/dd6d367d-c356-4785-af8c-66973ef6d3fb/summary?pipeline_id=791ce31f-24f7-46f4-8503-b220f5b21c61