Copilot commented on code in PR #8676:
URL: https://github.com/apache/hbase/pull/8676#discussion_r4067574633
##########
dev-support/read-replica/python/src/hbase_docker_client.py:
##########
@@ -51,22 +50,20 @@ class HBaseInitializationError(Exception):
class HBaseDockerClient:
def __init__(self, container_name: str, local_conf: str, hbase_ui_port:
int = 16010,
- cluster_name: str = "HBase Cluster", max_retries: int = 12,
sleep_time: int = 5,
- hbase_host: str = "localhost") -> None:
+ cluster_name: str = "HBase Cluster", max_retries: int = 12,
sleep_time: int = 5) -> None:
self._container_name = container_name
self._local_conf = local_conf
self._hbase_ui_port = hbase_ui_port
self._cluster_name = cluster_name
self._max_retries = max_retries
self._sleep_time = sleep_time
- self._hbase_host = hbase_host
self._docker_client = docker.from_env()
@property
def name(self) -> str:
return self._cluster_name
- def run_docker_exec_command(self, bash_cmd: str, timeout: int | None =
None) -> str:
+ def run_docker_exec_command(self, bash_cmd: str, timeout: int | None =
None) -> str | None:
Review Comment:
The annotation now permits `None`, but every successful path converts stdout
with `(stdout or b'').decode(...)` and returns a `str`; failure paths raise.
Keeping `-> str` accurately documents the contract and remains consistent with
callers such as `run_hbase_shell_command`, which also promises `str`.
##########
dev-support/read-replica/.env:
##########
@@ -19,19 +19,11 @@
HBASE_IMAGE=${HBASE_IMAGE:-kgeisz/hbase-docker:read-replica-jenkins}
# The name of the HBase docker container
HBASE_CONTAINER_NAME=hbase-docker
-# This is the host running the hbase-docker containers. Use localhost if the
containers
-# are running locally. If they are started by another container, such as a
Jenkins
-# container in a Docker-out-of-Docker setup, then try setting this to
host.docker.internal.
-HBASE_HOST=${HBASE_HOST:-localhost}
# The directory within the docker container that contains the config files
Review Comment:
Removing the two port variables here leaves
`dev-support/read-replica/README.md:194` documenting `ACTIVE_CLUSTER_PORT` and
`REPLICA_CLUSTER_PORT` as available `.env` settings. Please remove or replace
that stale bullet so the updated documentation matches the new
container-internal readiness check.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
