fgerlits commented on a change in pull request #1121:
URL: https://github.com/apache/nifi-minifi-cpp/pull/1121#discussion_r697296188



##########
File path: docker/test/integration/minifi/core/Container.py
##########
@@ -0,0 +1,47 @@
+import docker
+import logging
+
+
+class Container:
+    def __init__(self, name, engine, vols, network, image_store):
+        self.name = name
+        self.engine = engine
+        self.vols = vols
+        self.network = network
+        self.image_store = image_store
+
+        # Get docker client
+        self.client = docker.from_env()
+        self.deployed = False
+
+    def __del__(self):
+        self.cleanup()
+
+    def cleanup(self):
+        logging.info('Cleaning up container: %s', self.name)
+        try:
+            self.client.containers.get(self.name).remove(v=True, force=True)
+        except docker.errors.NotFound:
+            logging.warn("Container '%s' has been cleaned up already, nothing 
to be done.", self.name)

Review comment:
       minor, but `warn()` is deprecated, `warning()` should be used instead

##########
File path: docker/test/integration/minifi/core/SingleNodeDockerCluster.py
##########
@@ -20,364 +19,73 @@ class SingleNodeDockerCluster(Cluster):
     testing or use-cases which do not span multiple compute nodes.
     """
 
-    def __init__(self):
-        self.minifi_version = os.environ['MINIFI_VERSION']
-        self.nifi_version = '1.7.0'
-        self.engine = 'minifi-cpp'
-        self.start_nodes = []
-        self.name = None
+    def __init__(self, image_store):
         self.vols = {}
-        self.minifi_root = '/opt/minifi/nifi-minifi-cpp-' + self.minifi_version
-        self.nifi_root = '/opt/nifi/nifi-' + self.nifi_version
-        self.kafka_broker_root = '/opt/kafka'
-        self.network = None
-        self.containers = OrderedDict()
-        self.images = []
-        self.tmp_files = []
+        self.network = self.create_docker_network()
+        self.containers = {}
+        self.image_store = image_store
+        self.data_directories = {}
 
         # Get docker client
         self.client = docker.from_env()
 
     def __del__(self):
-        """
-        Clean up ephemeral cluster resources
-        """
-
-        # Containers and networks are expected to be freed outside of this 
function
-
-        # Clean up images
-        for image in reversed(self.images):
-            logging.info('Cleaning up image: %s', image[0].id)
-            self.client.images.remove(image[0].id, force=True)
-
-        # Clean up tmp files
-        for tmp_file in self.tmp_files:
-            os.remove(tmp_file)
-
-    def set_name(self, name):
-        self.name = name
-
-    def get_name(self):
-        return self.name
-
-    def set_engine(self, engine):
-        self.engine = engine
-
-    def get_engine(self):
-        return self.engine
-
-    def get_start_nodes(self):
-        return self.start_nodes
-
-    def add_start_node(self, node):
-        self.start_nodes.append(node)
-
-    def set_directory_bindings(self, bindings):
-        self.vols = bindings
+        self.cleanup()
+
+    def cleanup(self):
+        for container in self.containers.values():
+            container.cleanup()
+        self.containers = {}
+        if self.network:
+            logging.info('Cleaning up network: %s', self.network.name)
+            self.network.remove()
+            self.network = None
+
+    def set_directory_bindings(self, volumes, data_directories):
+        self.vols = volumes
+        self.data_directories = data_directories
+        for container in self.containers.values():
+            container.vols = self.vols
 
     @staticmethod
     def create_docker_network():
         net_name = 'minifi_integration_test_network-' + str(uuid.uuid4())
-        logging.info('Creating network: %s', net_name)
+        logging.debug('Creating network: %s', net_name)
         return docker.from_env().networks.create(net_name)
 
-    def set_network(self, network):
-        self.network = network
-
-    def deploy_flow(self):
-        """
-        Compiles the flow to a valid config file and overlays it into a new 
image.
-        """
-
-        if self.vols is None:
-            self.vols = {}
-
-        if self.name is None:
-            self.name = self.engine + '-' + str(uuid.uuid4())
-            logging.info('Flow name was not provided; using generated name 
\'%s\'', self.name)
-
-        logging.info('Deploying %s flow \"%s\"...', self.engine, self.name)
-
-        # Create network if necessary
-        if self.network is None:
-            self.set_network(self.create_docker_network())
-
-        if self.engine == 'nifi':
-            self.deploy_nifi_flow()
-        elif self.engine == 'minifi-cpp':
-            self.deploy_minifi_cpp_flow()
-        elif self.engine == 'kafka-broker':
-            self.deploy_kafka_broker()
-        elif self.engine == 'http-proxy':
-            self.deploy_http_proxy()
-        elif self.engine == 's3-server':
-            self.deploy_s3_server()
-        elif self.engine == 'azure-storage-server':
-            self.deploy_azure_storage_server()
-        elif self.engine == 'postgresql-server':
-            self.deploy_postgres_server()
+    def acquire_container(self, name, engine='minifi-cpp'):
+        if name is not None and name in self.containers:
+            return self.containers[name]
+
+        if name is None and (engine == 'nifi' or engine == 'minifi-cpp'):
+            name = engine + '-' + str(uuid.uuid4())
+            logging.info('Container name was not provided; using generated 
name \'%s\'', self.name)

Review comment:
       this should be just `name` instead of `self.name`, right?
   
   same applies to `self.engine` in line 81

##########
File path: docker/test/integration/minifi/processors/FetchS3Object.py
##########
@@ -20,4 +20,4 @@ def __init__(self,
                 'Proxy Username': proxy_username,
                 'Proxy Password': proxy_password,
             },
-            auto_terminate=['success'])
+            auto_terminate=['failure'])

Review comment:
       why did this change?

##########
File path: docker/test/integration/minifi/core/ImageStore.py
##########
@@ -0,0 +1,183 @@
+from .NifiContainer import NifiContainer
+from .MinifiContainer import MinifiContainer
+import logging
+import tarfile
+import docker
+from io import BytesIO
+from textwrap import dedent
+import os
+
+
+class ImageStore:
+    def __init__(self):
+        self.client = docker.from_env()
+        self.images = dict()
+
+    def __del__(self):
+        self.cleanup()
+
+    def cleanup(self):
+        # Clean up images
+        for image_id in self.images:

Review comment:
       why not `for image in self.images.values()`?




-- 
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: issues-unsubscr...@nifi.apache.org

For queries about this service, please contact Infrastructure at:
us...@infra.apache.org


Reply via email to