lordgamez commented on code in PR #2044: URL: https://github.com/apache/nifi-minifi-cpp/pull/2044#discussion_r2444917902
########## behave_framework/src/minifi_test_framework/containers/container.py: ########## @@ -0,0 +1,266 @@ +# +# Licensed to the Apache Software Foundation (ASF) under one or more +# contributor license agreements. See the NOTICE file distributed with +# this work for additional information regarding copyright ownership. +# The ASF licenses this file to You under the Apache License, Version 2.0 +# (the "License"); you may not use this file except in compliance with +# the License. You may obtain a copy of the License at +# +# http://www.apache.org/licenses/LICENSE-2.0 +# +# Unless required by applicable law or agreed to in writing, software +# distributed under the License is distributed on an "AS IS" BASIS, +# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +# See the License for the specific language governing permissions and +# limitations under the License. +# + +import logging +import os +import shlex +import tempfile +from docker.models.networks import Network +from minifi_test_framework.containers.directory import Directory +from minifi_test_framework.containers.file import File +from minifi_test_framework.containers.host_file import HostFile +from typing import Dict, Any, List + +import docker + + +class Container: + def __init__(self, image_name: str, container_name: str, network: Network): + self.image_name: str = image_name + self.container_name: str = container_name + self.network: Network = network + self.user: str = "0:0" + self.client = docker.from_env() + self.container = None + self.files: List[File] = [] + self.dirs: List[Directory] = [] + self.host_files: List[HostFile] = [] + self.volumes = {} + self.command = None + self._temp_dir = None + self.ports = None + self.environment: List[str] = [] + + def deploy(self) -> bool: + self._temp_dir = tempfile.TemporaryDirectory() + + if len(self.files) != 0 or len(self.dirs) != 0 or len(self.host_files) != 0: + for file in self.files: + temp_path = self._temp_dir.name + "/" + file.host_filename + with open(temp_path, "w") as temp_file: + temp_file.write(file.content) + self.volumes[temp_path] = { + "bind": file.path + "/" + file.host_filename, + "mode": file.mode + } + for directory in self.dirs: + temp_path = self._temp_dir.name + directory.path + for file_name, content in directory.files.items(): + file_path = temp_path + "/" + file_name + os.makedirs(temp_path, exist_ok=True) + with open(file_path, "w") as temp_file: + temp_file.write(content) + self.volumes[temp_path] = { + "bind": directory.path, + "mode": directory.mode + } + for host_file in self.host_files: + self.volumes[host_file.container_path] = { + "bind": host_file.host_path, + "mode": host_file.mode + } + + try: + existing_container = self.client.containers.get(self.container_name) + logging.warning(f"Found existing container '{self.container_name}'. Removing it first.") + existing_container.remove(force=True) + except docker.errors.NotFound: + pass + try: + print(f"Creating and starting container '{self.container_name}'...") Review Comment: Why do we print here instead of using logging? ########## behave_framework/src/minifi_test_framework/containers/container.py: ########## @@ -0,0 +1,266 @@ +# +# Licensed to the Apache Software Foundation (ASF) under one or more +# contributor license agreements. See the NOTICE file distributed with +# this work for additional information regarding copyright ownership. +# The ASF licenses this file to You under the Apache License, Version 2.0 +# (the "License"); you may not use this file except in compliance with +# the License. You may obtain a copy of the License at +# +# http://www.apache.org/licenses/LICENSE-2.0 +# +# Unless required by applicable law or agreed to in writing, software +# distributed under the License is distributed on an "AS IS" BASIS, +# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +# See the License for the specific language governing permissions and +# limitations under the License. +# + +import logging +import os +import shlex +import tempfile +from docker.models.networks import Network +from minifi_test_framework.containers.directory import Directory +from minifi_test_framework.containers.file import File +from minifi_test_framework.containers.host_file import HostFile +from typing import Dict, Any, List + +import docker + + +class Container: + def __init__(self, image_name: str, container_name: str, network: Network): + self.image_name: str = image_name + self.container_name: str = container_name + self.network: Network = network + self.user: str = "0:0" + self.client = docker.from_env() + self.container = None + self.files: List[File] = [] + self.dirs: List[Directory] = [] + self.host_files: List[HostFile] = [] + self.volumes = {} + self.command = None + self._temp_dir = None + self.ports = None + self.environment: List[str] = [] + + def deploy(self) -> bool: + self._temp_dir = tempfile.TemporaryDirectory() + + if len(self.files) != 0 or len(self.dirs) != 0 or len(self.host_files) != 0: Review Comment: Do we need this check if we just iterate through all of the files? If those lists are empty we don't do anything anyway ########## extensions/standard-processors/tests/features/attributes_to_json.feature: ########## @@ -15,17 +15,16 @@ @CORE Feature: Writing attribute data using AttributesToJSON processor - Background: - Given the content of "/tmp/output" is monitored Scenario: Write selected attribute data to file Given a GetFile processor with the "Input Directory" property set to "/tmp/input" - And a file with filename "test_file.log" and content "test_data" is present in "/tmp/input" + And a directory at "/tmp/input" has a file ("test_file.log") with the content "test_data" And a AttributesToJSON processor with the "Attributes List" property set to "filename,invalid" And the "Destination" property of the AttributesToJSON processor is set to "flowfile-content" And the "Null Value" property of the AttributesToJSON processor is set to "true" And a PutFile processor with the "Directory" property set to "/tmp/output" And the "success" relationship of the GetFile processor is connected to the AttributesToJSON And the "success" relationship of the AttributesToJSON processor is connected to the PutFile + And PutFile's success relationship is auto-terminated When the MiNiFi instance starts up - Then a flowfile with the JSON content "{"filename":"test_file.log","invalid":null}" is placed in the monitored directory in less than 10 seconds + Then there is a single file with "{"invalid":null,"filename":"test_file.log"}" content in the "/tmp/output" directory in less than 20 seconds Review Comment: I would keep the JSON content check instead of raw file content check as the JSON field order is not guaranteed. ########## .github/workflows/ci.yml: ########## @@ -535,6 +535,64 @@ jobs: with: name: behavex_output path: build/behavex_output + modular_docker_tests: + name: "Modular Docker integration tests (x86_64)" + needs: docker_build + runs-on: ubuntu-24.04 + timeout-minutes: 180 + steps: + - id: checkout + uses: actions/checkout@v4 + - id: run_cmake + name: Run CMake + run: | + mkdir build + cd build + cmake ${DOCKER_CMAKE_FLAGS} .. + - name: Download artifact + uses: actions/download-artifact@v4 + with: + name: minifi_docker + path: build + - name: Load Docker image + run: | + docker load --input ./build/minifi_docker.tar + - id: install_deps + name: Install dependencies for Docker Verify + run: | + sudo apt update + sudo apt install -y python3-virtualenv + - id: free_disk_space + run: | + # We can gain additional disk space on the Ubuntu runners thanks to these suggestions: + # https://github.com/actions/runner-images/issues/2840#issuecomment-790492173 + # https://github.com/actions/runner-images/issues/2606#issuecomment-772683150 + sudo rm -rf /usr/share/dotnet + sudo rm -rf /usr/local/lib/android + sudo rm -rf /opt/ghc + sudo rm -rf "/usr/local/share/boost" + sudo rm -rf "$AGENT_TOOLSDIRECTORY" + - id: test + name: Docker Verify + working-directory: ./build + run: make docker-verify-modular Review Comment: I think we should document how the modular docker tests should be run using the make target, and also manually a specific feature set. We should also document the required python environment and the required python version. In some places we are using the `|` pipe character which would require python 3.10, but other times we are using Optional instead which can be used with python 3.9. We should decide and if we require python 3.10 then maybe we may not need the `typing` package at all but we could use the built in type hints. ########## behave_framework/src/minifi_test_framework/containers/docker_container_builder.py: ########## @@ -0,0 +1,88 @@ +# +# Licensed to the Apache Software Foundation (ASF) under one or more +# contributor license agreements. See the NOTICE file distributed with +# this work for additional information regarding copyright ownership. +# The ASF licenses this file to You under the Apache License, Version 2.0 +# (the "License"); you may not use this file except in compliance with +# the License. You may obtain a copy of the License at +# +# http://www.apache.org/licenses/LICENSE-2.0 +# +# Unless required by applicable law or agreed to in writing, software +# distributed under the License is distributed on an "AS IS" BASIS, +# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +# See the License for the specific language governing permissions and +# limitations under the License. +# + +import logging +import os +import tempfile +from docker.models.images import Image +from typing import Optional + +import docker Review Comment: nitpick, but I would move it up the top next to the other imports ########## behave_framework/src/minifi_test_framework/containers/container.py: ########## @@ -0,0 +1,266 @@ +# +# Licensed to the Apache Software Foundation (ASF) under one or more +# contributor license agreements. See the NOTICE file distributed with +# this work for additional information regarding copyright ownership. +# The ASF licenses this file to You under the Apache License, Version 2.0 +# (the "License"); you may not use this file except in compliance with +# the License. You may obtain a copy of the License at +# +# http://www.apache.org/licenses/LICENSE-2.0 +# +# Unless required by applicable law or agreed to in writing, software +# distributed under the License is distributed on an "AS IS" BASIS, +# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +# See the License for the specific language governing permissions and +# limitations under the License. +# + +import logging +import os +import shlex +import tempfile +from docker.models.networks import Network +from minifi_test_framework.containers.directory import Directory +from minifi_test_framework.containers.file import File +from minifi_test_framework.containers.host_file import HostFile +from typing import Dict, Any, List + +import docker + + +class Container: + def __init__(self, image_name: str, container_name: str, network: Network): + self.image_name: str = image_name + self.container_name: str = container_name + self.network: Network = network + self.user: str = "0:0" + self.client = docker.from_env() + self.container = None + self.files: List[File] = [] + self.dirs: List[Directory] = [] + self.host_files: List[HostFile] = [] + self.volumes = {} + self.command = None + self._temp_dir = None + self.ports = None + self.environment: List[str] = [] + + def deploy(self) -> bool: + self._temp_dir = tempfile.TemporaryDirectory() + + if len(self.files) != 0 or len(self.dirs) != 0 or len(self.host_files) != 0: + for file in self.files: + temp_path = self._temp_dir.name + "/" + file.host_filename + with open(temp_path, "w") as temp_file: + temp_file.write(file.content) + self.volumes[temp_path] = { + "bind": file.path + "/" + file.host_filename, + "mode": file.mode + } + for directory in self.dirs: + temp_path = self._temp_dir.name + directory.path + for file_name, content in directory.files.items(): + file_path = temp_path + "/" + file_name + os.makedirs(temp_path, exist_ok=True) + with open(file_path, "w") as temp_file: + temp_file.write(content) + self.volumes[temp_path] = { + "bind": directory.path, + "mode": directory.mode + } + for host_file in self.host_files: + self.volumes[host_file.container_path] = { + "bind": host_file.host_path, + "mode": host_file.mode + } + + try: + existing_container = self.client.containers.get(self.container_name) + logging.warning(f"Found existing container '{self.container_name}'. Removing it first.") + existing_container.remove(force=True) + except docker.errors.NotFound: + pass + try: + print(f"Creating and starting container '{self.container_name}'...") + self.container = self.client.containers.run( + image=self.image_name, + name=self.container_name, + ports=self.ports, + environment=self.environment, + volumes=self.volumes, + network=self.network.name, + command=self.command, + user=self.user, + detach=True # Always run in the background + ) + except Exception as e: + logging.error(f"Error starting container: {e}") + raise + return True + + def clean_up(self): + if self.container is not None: Review Comment: nitpick, but we could keep it consistent with the other check by just using `if not self.container:` here too ########## behave_framework/src/minifi_test_framework/steps/core_steps.py: ########## @@ -0,0 +1,55 @@ +# +# Licensed to the Apache Software Foundation (ASF) under one or more +# contributor license agreements. See the NOTICE file distributed with +# this work for additional information regarding copyright ownership. +# The ASF licenses this file to You under the Apache License, Version 2.0 +# (the "License"); you may not use this file except in compliance with +# the License. You may obtain a copy of the License at +# +# http://www.apache.org/licenses/LICENSE-2.0 +# +# Unless required by applicable law or agreed to in writing, software +# distributed under the License is distributed on an "AS IS" BASIS, +# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +# See the License for the specific language governing permissions and +# limitations under the License. +# + +import humanfriendly +import logging +import random +import string +from behave import when, step +from minifi_test_framework.containers.directory import Directory +from minifi_test_framework.containers.file import File +from minifi_test_framework.core.minifi_test_context import MinifiTestContext + + +@when("both instances start up") +@when("all instances start up") +def step_impl(context: MinifiTestContext): + for container in context.containers: + assert container.deploy() + assert context.minifi_container.deploy() or context.minifi_container.get_logs() + logging.debug("All instances started up") + + +@when("the MiNiFi instance starts up") +def step_impl(context): + assert context.minifi_container.deploy() + logging.debug("All instances started up") Review Comment: I think this log doesn't belong here ########## behave_framework/src/minifi_test_framework/containers/docker_container_builder.py: ########## @@ -0,0 +1,88 @@ +# +# Licensed to the Apache Software Foundation (ASF) under one or more +# contributor license agreements. See the NOTICE file distributed with +# this work for additional information regarding copyright ownership. +# The ASF licenses this file to You under the Apache License, Version 2.0 +# (the "License"); you may not use this file except in compliance with +# the License. You may obtain a copy of the License at +# +# http://www.apache.org/licenses/LICENSE-2.0 +# +# Unless required by applicable law or agreed to in writing, software +# distributed under the License is distributed on an "AS IS" BASIS, +# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +# See the License for the specific language governing permissions and +# limitations under the License. +# + +import logging +import os +import tempfile +from docker.models.images import Image +from typing import Optional + +import docker + + +class DockerContainerBuilder: Review Comment: I would rename this DockerImageBuilder as it builds images not containers ########## behave_framework/src/minifi_test_framework/containers/container.py: ########## @@ -0,0 +1,266 @@ +# +# Licensed to the Apache Software Foundation (ASF) under one or more +# contributor license agreements. See the NOTICE file distributed with +# this work for additional information regarding copyright ownership. +# The ASF licenses this file to You under the Apache License, Version 2.0 +# (the "License"); you may not use this file except in compliance with +# the License. You may obtain a copy of the License at +# +# http://www.apache.org/licenses/LICENSE-2.0 +# +# Unless required by applicable law or agreed to in writing, software +# distributed under the License is distributed on an "AS IS" BASIS, +# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +# See the License for the specific language governing permissions and +# limitations under the License. +# + +import logging +import os +import shlex +import tempfile +from docker.models.networks import Network +from minifi_test_framework.containers.directory import Directory +from minifi_test_framework.containers.file import File +from minifi_test_framework.containers.host_file import HostFile +from typing import Dict, Any, List + +import docker + + +class Container: + def __init__(self, image_name: str, container_name: str, network: Network): + self.image_name: str = image_name + self.container_name: str = container_name + self.network: Network = network + self.user: str = "0:0" + self.client = docker.from_env() + self.container = None + self.files: List[File] = [] + self.dirs: List[Directory] = [] + self.host_files: List[HostFile] = [] + self.volumes = {} + self.command = None + self._temp_dir = None + self.ports = None + self.environment: List[str] = [] + + def deploy(self) -> bool: + self._temp_dir = tempfile.TemporaryDirectory() + + if len(self.files) != 0 or len(self.dirs) != 0 or len(self.host_files) != 0: + for file in self.files: + temp_path = self._temp_dir.name + "/" + file.host_filename + with open(temp_path, "w") as temp_file: + temp_file.write(file.content) + self.volumes[temp_path] = { + "bind": file.path + "/" + file.host_filename, + "mode": file.mode + } + for directory in self.dirs: + temp_path = self._temp_dir.name + directory.path + for file_name, content in directory.files.items(): + file_path = temp_path + "/" + file_name + os.makedirs(temp_path, exist_ok=True) + with open(file_path, "w") as temp_file: + temp_file.write(content) + self.volumes[temp_path] = { + "bind": directory.path, + "mode": directory.mode + } + for host_file in self.host_files: + self.volumes[host_file.container_path] = { + "bind": host_file.host_path, + "mode": host_file.mode + } + + try: + existing_container = self.client.containers.get(self.container_name) + logging.warning(f"Found existing container '{self.container_name}'. Removing it first.") + existing_container.remove(force=True) + except docker.errors.NotFound: + pass Review Comment: We could write a warning here ########## extensions/aws/tests/features/s3.feature: ########## @@ -19,89 +19,74 @@ Feature: Sending data from MiNiFi-C++ to an AWS server As a user of MiNiFi I need to have PutS3Object and DeleteS3Object processors - Background: - Given the content of "/tmp/output" is monitored - Scenario: A MiNiFi instance transfers encoded data to s3 Given a GetFile processor with the "Input Directory" property set to "/tmp/input" - And a file with the content "LH_O#L|FD<FASD{FO#@$#$%^ \"#\"$L%:\"@#$L\":test_data#$#%#$%?{\"F{" is present in "/tmp/input" - And a PutS3Object processor set up to communicate with an s3 server - And a PutFile processor with the "Directory" property set to "/tmp/output" - And the "success" relationship of the GetFile processor is connected to the PutS3Object - And the "success" relationship of the PutS3Object processor is connected to the PutFile - - And a s3 server is set up in correspondence with the PutS3Object - - When both instances start up - - Then a flowfile with the content "test" is placed in the monitored directory in less than 60 seconds - And the object on the s3 server is "LH_O#L|FD<FASD{FO#@$#$%^ \"#\"$L%:\"@#$L\":test_data#$#%#$%?{\"F{" - And the object content type on the s3 server is "application/octet-stream" and the object metadata matches use metadata - And the Minifi logs contain the following message: "in a single upload" in less than 10 seconds - - Scenario: A MiNiFi instance transfers encoded data to s3 in FIPS mode - Given OpenSSL FIPS mode is enabled in MiNiFi - And a GetFile processor with the "Input Directory" property set to "/tmp/input" - And a file with the content "LH_O#L|FD<FASD{FO#@$#$%^ \"#\"$L%:\"@#$L\":test_data#$#%#$%?{\"F{" is present in "/tmp/input" + And a directory at "/tmp/input" has a file with the content "LH_O#L|FD<FASD{FO#@$#$%^ \"#\"$L%:\"@#$L\":test_data#$#%#$%?{\"F{" And a PutS3Object processor set up to communicate with an s3 server And a PutFile processor with the "Directory" property set to "/tmp/output" And the "success" relationship of the GetFile processor is connected to the PutS3Object And the "success" relationship of the PutS3Object processor is connected to the PutFile + And the "failure" relationship of the PutS3Object processor is connected to the PutS3Object + And PutFile's success relationship is auto-terminated - And a s3 server is set up in correspondence with the PutS3Object + And an s3 server is set up in correspondence with the PutS3Object When both instances start up - Then a flowfile with the content "test" is placed in the monitored directory in less than 60 seconds + Then there is a single file with "LH_O#L|FD<FASD{FO#@$#$%^ \"#\"$L%:\"@#$L\":test_data#$#%#$%?{\"F{" content in the "/tmp/output" directory in less than 20 seconds And the object on the s3 server is "LH_O#L|FD<FASD{FO#@$#$%^ \"#\"$L%:\"@#$L\":test_data#$#%#$%?{\"F{" And the object content type on the s3 server is "application/octet-stream" and the object metadata matches use metadata And the Minifi logs contain the following message: "in a single upload" in less than 10 seconds Scenario: A MiNiFi instance transfers encoded data through a http proxy to s3 Given a GetFile processor with the "Input Directory" property set to "/tmp/input" - And a file with the content "LH_O#L|FD<FASD{FO#@$#$%^ \"#\"$L%:\"@#$L\":test_data#$#%#$%?{\"F{" is present in "/tmp/input" + And a directory at "/tmp/input" has a file with the content "LH_O#L|FD<FASD{FO#@$#$%^ \"#\"$L%:\"@#$L\":test_data#$#%#$%?{\"F{" And a PutS3Object processor set up to communicate with an s3 server - And these processor properties are set to match the http proxy: - | processor name | property name | property value | - | PutS3Object | Proxy Host | http-proxy-${feature_id} | - | PutS3Object | Proxy Port | 3128 | - | PutS3Object | Proxy Username | admin | - | PutS3Object | Proxy Password | test101 | + And these processor properties are set + | processor name | property name | property value | + | PutS3Object | Proxy Host | http-proxy-s3-1 | Review Comment: Would it be possible to substitute the scenario name automatically instead of figuring it out manually when writing the test? If there are a large number of test scenarios it would be cumbersome and error prone to figure out the current index of the scenario, or if an additional scenario is added in between it would need to be changed. ########## behave_framework/src/minifi_test_framework/core/hooks.py: ########## @@ -0,0 +1,59 @@ +# Licensed to the Apache Software Foundation (ASF) under one or more +# contributor license agreements. See the NOTICE file distributed with +# this work for additional information regarding copyright ownership. +# The ASF licenses this file to You under the Apache License, Version 2.0 +# (the "License"); you may not use this file except in compliance with +# the License. You may obtain a copy of the License at +# +# http://www.apache.org/licenses/LICENSE-2.0 +# +# Unless required by applicable law or agreed to in writing, software +# distributed under the License is distributed on an "AS IS" BASIS, +# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +# See the License for the specific language governing permissions and +# limitations under the License. + + +import logging +import os +from behave.model import Scenario +from behave.runner import Context +from minifi_test_framework.containers.minifi_container import MinifiContainer +from minifi_test_framework.core.minifi_test_context import MinifiTestContext + +import docker Review Comment: nitpick, could be moved up to the first imports ########## behave_framework/src/minifi_test_framework/containers/container.py: ########## @@ -0,0 +1,266 @@ +# +# Licensed to the Apache Software Foundation (ASF) under one or more +# contributor license agreements. See the NOTICE file distributed with +# this work for additional information regarding copyright ownership. +# The ASF licenses this file to You under the Apache License, Version 2.0 +# (the "License"); you may not use this file except in compliance with +# the License. You may obtain a copy of the License at +# +# http://www.apache.org/licenses/LICENSE-2.0 +# +# Unless required by applicable law or agreed to in writing, software +# distributed under the License is distributed on an "AS IS" BASIS, +# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +# See the License for the specific language governing permissions and +# limitations under the License. +# + +import logging +import os +import shlex +import tempfile +from docker.models.networks import Network +from minifi_test_framework.containers.directory import Directory +from minifi_test_framework.containers.file import File +from minifi_test_framework.containers.host_file import HostFile +from typing import Dict, Any, List + +import docker + + +class Container: + def __init__(self, image_name: str, container_name: str, network: Network): + self.image_name: str = image_name + self.container_name: str = container_name + self.network: Network = network + self.user: str = "0:0" + self.client = docker.from_env() + self.container = None + self.files: List[File] = [] + self.dirs: List[Directory] = [] + self.host_files: List[HostFile] = [] + self.volumes = {} + self.command = None + self._temp_dir = None + self.ports = None + self.environment: List[str] = [] + + def deploy(self) -> bool: + self._temp_dir = tempfile.TemporaryDirectory() + + if len(self.files) != 0 or len(self.dirs) != 0 or len(self.host_files) != 0: + for file in self.files: + temp_path = self._temp_dir.name + "/" + file.host_filename + with open(temp_path, "w") as temp_file: + temp_file.write(file.content) + self.volumes[temp_path] = { + "bind": file.path + "/" + file.host_filename, + "mode": file.mode + } + for directory in self.dirs: + temp_path = self._temp_dir.name + directory.path + for file_name, content in directory.files.items(): + file_path = temp_path + "/" + file_name + os.makedirs(temp_path, exist_ok=True) + with open(file_path, "w") as temp_file: + temp_file.write(content) + self.volumes[temp_path] = { + "bind": directory.path, + "mode": directory.mode + } + for host_file in self.host_files: + self.volumes[host_file.container_path] = { + "bind": host_file.host_path, + "mode": host_file.mode + } + + try: + existing_container = self.client.containers.get(self.container_name) + logging.warning(f"Found existing container '{self.container_name}'. Removing it first.") + existing_container.remove(force=True) + except docker.errors.NotFound: + pass + try: + print(f"Creating and starting container '{self.container_name}'...") + self.container = self.client.containers.run( + image=self.image_name, + name=self.container_name, + ports=self.ports, + environment=self.environment, + volumes=self.volumes, + network=self.network.name, + command=self.command, + user=self.user, + detach=True # Always run in the background + ) + except Exception as e: + logging.error(f"Error starting container: {e}") + raise + return True + + def clean_up(self): + if self.container is not None: + self.container.remove(force=True) + + def exec_run(self, command): Review Comment: We should define a type hint for the return value like `-> Tuple[int | None, str] ########## behave_framework/src/minifi_test_framework/core/helpers.py: ########## @@ -0,0 +1,58 @@ +# +# Licensed to the Apache Software Foundation (ASF) under one or more +# contributor license agreements. See the NOTICE file distributed with +# this work for additional information regarding copyright ownership. +# The ASF licenses this file to You under the Apache License, Version 2.0 +# (the "License"); you may not use this file except in compliance with +# the License. You may obtain a copy of the License at +# +# http://www.apache.org/licenses/LICENSE-2.0 +# +# Unless required by applicable law or agreed to in writing, software +# distributed under the License is distributed on an "AS IS" BASIS, +# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +# See the License for the specific language governing permissions and +# limitations under the License. +# + +from __future__ import annotations + +import logging +import time +from minifi_test_framework.core.minifi_test_context import MinifiTestContext +from typing import Callable + + +def log_due_to_failure(context: MinifiTestContext | None): + if context is not None: + for container in context.containers: + container.log_app_output() + context.minifi_container.log_app_output() + + +def wait_for_condition(condition: Callable[[], bool], timeout_seconds: float, bail_condition: Callable[[], bool], + context: MinifiTestContext | None) -> bool: + if bail_condition(): + logging.warning("Bail condition evaluated to 'True', aborting wait.") + log_due_to_failure(context) + return False + start_time = time.monotonic() + try: + while time.monotonic() - start_time < timeout_seconds: + if condition(): + return True + if bail_condition(): + logging.warning("Bail condition evaluated to 'True', aborting wait.") + log_due_to_failure(context) + return False + remaining_time = timeout_seconds - (time.monotonic() - start_time) + sleep_time = min(timeout_seconds / 10, remaining_time) + if sleep_time > 0: + time.sleep(sleep_time) + except (Exception,): Review Comment: Could this be just `except Exception:`? -- 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]
