Merge pull request Azure#15 from daniv-msft/harrli/lint-fix

Linting fix

src/containerapp/azext_containerapp/_up_utils.py

@@ -7,8 +7,8 @@
 
 from tempfile import NamedTemporaryFile
 from urllib.parse import urlparse
-import requests
 import subprocess
+import requests
 
 from azure.cli.core.azclierror import (
     RequiredArgumentMissingError,
@@ -359,7 +359,7 @@ def create_acr(self):
             self.cmd.cli_ctx, registry_name
         )
 
-    def build_container_from_source_with_buildpack(self, image_name, source):
+    def build_container_from_source_with_buildpack(self, image_name, source):  # pylint: disable=too-many-statements
         # Ensure that Docker is running
         if not is_docker_running():
             raise ValidationError("Docker is not running. Please start Docker to use buildpacks.")
@@ -379,13 +379,13 @@ def build_container_from_source_with_buildpack(self, image_name, source):
         command = [pack_exec_path, 'config', 'default-builder', builder_image_name]
         logger.debug(f"Calling '{' '.join(command)}'")
         try:
-            process = subprocess.Popen(command, stdout=subprocess.PIPE, stderr=subprocess.PIPE)
-            stdout, stderr = process.communicate()
-            if process.returncode != 0:
-                raise CLIError(f"Error thrown when running 'pack config': {stderr.decode('utf-8')}")
-            logger.debug(f"Successfully set the default builder to {builder_image_name}.")
+            with subprocess.Popen(command, stdout=subprocess.PIPE, stderr=subprocess.PIPE) as process:
+                _, stderr = process.communicate()
+                if process.returncode != 0:
+                    raise CLIError(f"Error thrown when running 'pack config': {stderr.decode('utf-8')}")
+                logger.debug(f"Successfully set the default builder to {builder_image_name}.")
         except Exception as ex:
-            raise ValidationError(f"Unable to run 'pack config' command to set default builder: {ex}")
+            raise ValidationError(f"Unable to run 'pack config' command to set default builder: {ex}") from ex
 
         # Run 'pack build' to produce a runnable application image for the Container App
         command = [pack_exec_path, 'build', image_name, '--builder', builder_image_name, '--path', source]
@@ -398,45 +398,45 @@ def build_container_from_source_with_buildpack(self, image_name, source):
         logger.debug(f"Calling '{' '.join(command)}'")
         try:
             is_non_supported_platform = False
-            process = subprocess.Popen(command, stdout=subprocess.PIPE)
+            with subprocess.Popen(command, stdout=subprocess.PIPE) as process:
 
-            # Stream output of 'pack build' to warning stream
-            while process.stdout.readable():
-                line = process.stdout.readline()
-                if not line:
-                    break
+                # Stream output of 'pack build' to warning stream
+                while process.stdout.readable():
+                    line = process.stdout.readline()
+                    if not line:
+                        break
 
-                stdout_line = str(line.strip(), 'utf-8')
-                logger.warning(stdout_line)
-                if not is_non_supported_platform and "No buildpack groups passed detection" in stdout_line:
-                    is_non_supported_platform = True
+                    stdout_line = str(line.strip(), 'utf-8')
+                    logger.warning(stdout_line)
+                    if not is_non_supported_platform and "No buildpack groups passed detection" in stdout_line:
+                        is_non_supported_platform = True
 
-            # Update the result of process.returncode
-            process.communicate()
-            if is_non_supported_platform:
-                raise ValidationError("Current buildpacks do not support the platform targeted in the provided source code.")
+                # Update the result of process.returncode
+                process.communicate()
+                if is_non_supported_platform:
+                    raise ValidationError("Current buildpacks do not support the platform targeted in the provided source code.")
 
-            if process.returncode != 0:
-                raise CLIError(f"Non-zero exit code returned from 'pack build'; please check the above output for more details.")
+                if process.returncode != 0:
+                    raise CLIError("Non-zero exit code returned from 'pack build'; please check the above output for more details.")
 
-            logger.debug(f"Successfully built image {image_name} using buildpacks.")
+                logger.debug(f"Successfully built image {image_name} using buildpacks.")
         except ValidationError as ex:
             raise ex
         except Exception as ex:
-            raise CLIError(f"Unable to run 'pack build' command to produce runnable application image: {ex}")
+            raise CLIError(f"Unable to run 'pack build' command to produce runnable application image: {ex}") from ex
 
         # Run 'docker push' to push the image to the ACR
         command = ['docker', 'push', image_name]
         logger.debug(f"Calling '{' '.join(command)}'")
         logger.warning(f"Built image {image_name} locally using buildpacks, attempting to push to registry...")
         try:
-            process = subprocess.Popen(command, stdout=subprocess.PIPE, stderr=subprocess.PIPE)
-            stdout, stderr = process.communicate()
-            if process.returncode != 0:
-                raise CLIError(f"Error thrown when running 'docker push': {stderr.decode('utf-8')}")
-            logger.debug(f"Successfully pushed image {image_name} to ACR.")
+            with subprocess.Popen(command, stdout=subprocess.PIPE, stderr=subprocess.PIPE) as process:
+                _, stderr = process.communicate()
+                if process.returncode != 0:
+                    raise CLIError(f"Error thrown when running 'docker push': {stderr.decode('utf-8')}")
+                logger.debug(f"Successfully pushed image {image_name} to ACR.")
         except Exception as ex:
-            raise CLIError(f"Unable to run 'docker push' command to push image to ACR: {ex}")
+            raise CLIError(f"Unable to run 'docker push' command to push image to ACR: {ex}") from ex
 
     def build_container_from_source_with_acr_task(self, image_name, source):
         from azure.cli.command_modules.acr.task import acr_task_create, acr_task_run
@@ -498,7 +498,7 @@ def run_acr_build(self, dockerfile, source, quiet=False, build_from_source=False
             try:
                 # First try to build source using buildpacks
                 # Temporary fix: using run time tag as customer image tag
-                # Waiting for buildpacks side to fix this issue: https://github.com/buildpacks/pack/issues/1753
+                # Waiting for buildpacks side to fix this issue: https://github.com/buildpacks/pack/issues/1750
                 logger.warning("Attempting to build image using buildpacks...")
                 run_image_tag = get_latest_buildpack_run_tag("aspnet", "7.0")
                 if run_image_tag is not None:
@@ -509,7 +509,7 @@ def run_acr_build(self, dockerfile, source, quiet=False, build_from_source=False
             except ValidationError as e:
                 logger.warning(f"Unable to use buildpacks to build image from source: {e}\nFalling back to ACR Task...")
             except CLIError as e:
-                logger.error(f"Failed to use buildpacks to build image from source.")
+                logger.error("Failed to use buildpacks to build image from source.")
                 raise e
 
             # If we're unable to use the buildpack, build source using an ACR Task

src/containerapp/azext_containerapp/_utils.py

@@ -7,17 +7,16 @@
 import time
 import json
 import platform
-import docker
 import stat
 import io
 import os
-import requests
-import hashlib
-import packaging.version as SemVer
-import re
 import tarfile
 import zipfile
-
+import hashlib
+import re
+import requests
+import docker
+import packaging.version as SemVer
 
 from urllib.parse import urlparse
 from urllib.request import urlopen
@@ -43,7 +42,7 @@
                          LOG_ANALYTICS_RP, CONTAINER_APPS_RP, CHECK_CERTIFICATE_NAME_AVAILABILITY_TYPE, ACR_IMAGE_SUFFIX,
                          LOGS_STRING, PENDING_STATUS, SUCCEEDED_STATUS, UPDATING_STATUS)
 from ._models import (ContainerAppCustomDomainEnvelope as ContainerAppCustomDomainEnvelopeModel, ManagedCertificateEnvelop as ManagedCertificateEnvelopModel)
-from ._models import ImagePatchableCheck, OryxMarinerRunImgTagProperty
+from ._models import OryxMarinerRunImgTagProperty
 
 logger = get_logger(__name__)
 
@@ -1772,20 +1771,20 @@ def get_pack_exec_path():
 
         # Attempt to install the pack CLI
         url = f"https://github.com/buildpacks/pack/releases/download/{pack_cli_version}/{compressed_download_file_name}"
-        req = urlopen(url)
-        compressed_file = io.BytesIO(req.read())
-        if host_os == "Windows":
-            zip_file = zipfile.ZipFile(compressed_file)
-            for file in zip_file.namelist():
-                if file.endswith(exec_name):
-                    with open(exec_path, "wb") as f:
-                        f.write(zip_file.read(file))
-        else:
-            with tarfile.open(fileobj=compressed_file, mode="r:gz") as tar:
-                for tar_info in tar:
-                    if tar_info.isfile() and tar_info.name.endswith(exec_name):
-                        with open(exec_path, "wb") as f:
-                            f.write(tar.extractfile(tar_info).read())
+        with urlopen(url) as req:
+            compressed_file = io.BytesIO(req.read())
+            if host_os == "Windows":
+                with zipfile.ZipFile(compressed_file) as zip_file:
+                    for file in zip_file.namelist():
+                        if file.endswith(exec_name):
+                            with open(exec_path, "wb") as f:
+                                f.write(zip_file.read(file))
+            else:
+                with tarfile.open(fileobj=compressed_file, mode="r:gz") as tar:
+                    for tar_info in tar:
+                        if tar_info.isfile() and tar_info.name.endswith(exec_name):
+                            with open(exec_path, "wb") as f:
+                                f.write(tar.extractfile(tar_info).read())
 
         # Add executable permissions for the current user if they don't exist
         if not os.access(exec_path, os.X_OK):
@@ -1867,19 +1866,19 @@ def patchable_check(repo_tag_split: str, oryx_builder_run_img_tags, inspect_resu
 
 
 def get_current_mariner_tags() -> list(OryxMarinerRunImgTagProperty):
-    r = requests.get("https://mcr.microsoft.com/v2/oryx/builder/tags/list")
+    r = requests.get("https://mcr.microsoft.com/v2/oryx/builder/tags/list", timeout=30)
     tags = r.json()
     tag_list = {}
     # only keep entries that container keyword "mariner"
     tags = [tag for tag in tags["tags"] if "mariner" in tag]
-    for tag in tags:
+    for tag in tags:  # pylint: disable=too-many-nested-blocks
         tag_obj = parse_oryx_mariner_tag(tag)
         if tag_obj:
             major_minor_ver = str(tag_obj["version"].major) + "." + str(tag_obj["version"].minor)
             support = tag_obj["support"]
             framework = tag_obj["framework"]
             mariner_ver = tag_obj["marinerVersion"]
-            if framework in tag_list.keys():
+            if framework in tag_list:
                 if major_minor_ver in tag_list[framework].keys():
                     if support in tag_list[framework][major_minor_ver].keys():
                         if mariner_ver in tag_list[framework][major_minor_ver][support].keys():
@@ -1896,7 +1895,7 @@ def get_current_mariner_tags() -> list(OryxMarinerRunImgTagProperty):
     return tag_list
 
 
-def get_latest_buildpack_run_tag(framework, version, support = "lts", mariner_version = "cbl-mariner2.0"):
+def get_latest_buildpack_run_tag(framework, version, support="lts", mariner_version="cbl-mariner2.0"):
     tags = get_current_mariner_tags()
     try:
         return tags[framework][version][support][mariner_version][0]["fullTag"]

src/containerapp/azext_containerapp/custom.py

@@ -13,8 +13,6 @@
 import subprocess
 from concurrent.futures import ThreadPoolExecutor
 import requests
-import json
-import subprocess
 
 from azure.cli.core import telemetry as telemetry_core
 
@@ -4376,13 +4374,13 @@ def patch_get_image_inspection(pack_exec_path, img, info_list):
     if (img["imageName"].find("run-dotnet") != -1) and (img["imageName"].find("cbl-mariner") != -1):
         inspect_result = {"remote_info": {"run_images": [{"name": "mcr.microsoft.com/oryx/builder:" + img["imageName"].split(":")[-1]}]}, "image_name": img["imageName"], "targetContainerName": img["targetContainerName"], "targetContainerAppName": img["targetContainerAppName"], "targetContainerAppEnvironmentName": img["targetContainerAppEnvironmentName"], "targetResourceGroup": img["targetResourceGroup"]}
     else:
-        img_info = subprocess.Popen(pack_exec_path + " inspect-image " + img["imageName"] + " --output json", shell=True, stderr=subprocess.PIPE, stdout=subprocess.PIPE)
-        img_info_out, img_info_err = img_info.communicate()
-        if img_info_err.find(b"status code 401 Unauthorized") != -1 or img_info_err.find(b"unable to find image") != -1:
-            inspect_result = dict(remote_info=401, image_name=img["imageName"])
-        else:
-            inspect_result = json.loads(img_info_out)
-        inspect_result.update({"targetContainerName": img["targetContainerName"], "targetContainerAppName": img["targetContainerAppName"], "targetContainerAppEnvironmentName": img["targetContainerAppEnvironmentName"], "targetResourceGroup": img["targetResourceGroup"]})
+        with subprocess.Popen(pack_exec_path + " inspect-image " + img["imageName"] + " --output json", shell=True, stderr=subprocess.PIPE, stdout=subprocess.PIPE) as img_info:
+            img_info_out, img_info_err = img_info.communicate()
+            if img_info_err.find(b"status code 401 Unauthorized") != -1 or img_info_err.find(b"unable to find image") != -1:
+                inspect_result = dict(remote_info=401, image_name=img["imageName"])
+            else:
+                inspect_result = json.loads(img_info_out)
+            inspect_result.update({"targetContainerName": img["targetContainerName"], "targetContainerAppName": img["targetContainerAppName"], "targetContainerAppEnvironmentName": img["targetContainerAppEnvironmentName"], "targetResourceGroup": img["targetResourceGroup"]})
     info_list.append(inspect_result)
 
 
@@ -4403,7 +4401,7 @@ def patch_interactive(cmd, resource_group_name=None, managed_env=None, show_all=
     if without_unpatchable_results == []:
         return
     user_input = input("Do you want to apply all the patches or specify by id? (y/n/id)\n")
-    patch_apply(cmd, patchable_check_results, user_input, pack_exec_path)
+    patch_apply_handle_input(cmd, patchable_check_results, user_input, pack_exec_path)
 
 
 def patch_apply(cmd, resource_group_name=None, managed_env=None, show_all=False):
@@ -4422,10 +4420,10 @@ def patch_apply(cmd, resource_group_name=None, managed_env=None, show_all=False)
     print(patchable_check_results_json)
     if without_unpatchable_results == []:
         return
-    patch_apply(cmd, patchable_check_results, "y", pack_exec_path)
+    patch_apply_handle_input(cmd, patchable_check_results, "y", pack_exec_path)
 
 
-def patch_apply(cmd, patch_check_list, method, pack_exec_path):
+def patch_apply_handle_input(cmd, patch_check_list, method, pack_exec_path):
     m = method.strip().lower()
     # Track number of times patches were applied successfully.
     patch_apply_count = 0
@@ -4475,11 +4473,11 @@ def patch_apply(cmd, patch_check_list, method, pack_exec_path):
 def patch_cli_call(cmd, resource_group, container_app_name, container_name, target_image_name, new_run_image, pack_exec_path):
     try:
         print("Applying patch for container app: " + container_app_name + " container: " + container_name)
-        subprocess.run(f"{pack_exec_path} rebase -q {target_image_name} --run-image {new_run_image}", shell=True)
+        subprocess.run(f"{pack_exec_path} rebase -q {target_image_name} --run-image {new_run_image}", shell=True, check=True)
         new_target_image_name = target_image_name.split(":")[0] + ":" + new_run_image.split(":")[1]
-        subprocess.run(f"docker tag {target_image_name} {new_target_image_name}", shell=True)
+        subprocess.run(f"docker tag {target_image_name} {new_target_image_name}", shell=True, check=True)
         print(f"Publishing {new_target_image_name} to registry...")
-        subprocess.run(f"docker push -q {new_target_image_name}", shell=True)
+        subprocess.run(f"docker push -q {new_target_image_name}", shell=True, check=True)
         print("Patch applied and published successfully.\nNew image: " + new_target_image_name)
     except Exception:
         print("Error: Failed to apply patch and publish. Check if registry is logged in and has write access.")