From 7dc8fa76bc9bec66b77b098cfca2f99d10a3d1a6 Mon Sep 17 00:00:00 2001 From: Guy Lichtman <1395797+glicht@users.noreply.github.com> Date: Thu, 26 Nov 2020 13:21:16 +0200 Subject: [PATCH 1/8] fail on unexpected exception + handle mac os docker --- demisto_sdk/commands/lint/helpers.py | 3 +- demisto_sdk/commands/lint/linter.py | 58 ++++++++++++++++------------ 2 files changed, 36 insertions(+), 25 deletions(-) diff --git a/demisto_sdk/commands/lint/helpers.py b/demisto_sdk/commands/lint/helpers.py index 80d90d940d3..f8a5a5a1614 100644 --- a/demisto_sdk/commands/lint/helpers.py +++ b/demisto_sdk/commands/lint/helpers.py @@ -263,7 +263,8 @@ def add_tmp_lint_files(content_repo: git.Repo, pack_path: Path, lint_files: List added_modules.append(cur_path) yield except Exception as e: - logger.error(str(e)) + logger.error(f'add_tmp_lint_files unexpected exception: {str(e)}') + raise finally: # If we want to change handling of files after finishing - do it here pass diff --git a/demisto_sdk/commands/lint/linter.py b/demisto_sdk/commands/lint/linter.py index cb7e9af77a0..11e515c78ef 100644 --- a/demisto_sdk/commands/lint/linter.py +++ b/demisto_sdk/commands/lint/linter.py @@ -4,6 +4,7 @@ import json import logging import os +import traceback from copy import deepcopy from typing import Any, Dict, List, Optional, Tuple @@ -123,30 +124,34 @@ def run_dev_packages(self, no_flake8: bool, no_bandit: bool, no_mypy: bool, no_p # If not python pack - skip pack if skip: return self._pkg_lint_status - - # Locate mandatory files in pack path - for more info checkout the context manager LintFiles - with add_tmp_lint_files(content_repo=self._content_repo, # type: ignore - pack_path=self._pack_abs_dir, - lint_files=self._facts["lint_files"], - modules=modules, - pack_type=self._pkg_lint_status["pack_type"]): - # Run lint check on host - flake8, bandit, mypy - if self._pkg_lint_status["pack_type"] == TYPE_PYTHON: - self._run_lint_in_host(no_flake8=no_flake8, - no_bandit=no_bandit, - no_mypy=no_mypy, - no_vulture=no_vulture, - no_xsoar_linter=no_xsoar_linter) - - # Run lint and test check on pack docker image - if self._facts["docker_engine"]: - self._run_lint_on_docker_image(no_pylint=no_pylint, - no_test=no_test, - no_pwsh_analyze=no_pwsh_analyze, - no_pwsh_test=no_pwsh_test, - keep_container=keep_container, - test_xml=test_xml) - + try: + # Locate mandatory files in pack path - for more info checkout the context manager LintFiles + with add_tmp_lint_files(content_repo=self._content_repo, # type: ignore + pack_path=self._pack_abs_dir, + lint_files=self._facts["lint_files"], + modules=modules, + pack_type=self._pkg_lint_status["pack_type"]): + # Run lint check on host - flake8, bandit, mypy + if self._pkg_lint_status["pack_type"] == TYPE_PYTHON: + self._run_lint_in_host(no_flake8=no_flake8, + no_bandit=no_bandit, + no_mypy=no_mypy, + no_vulture=no_vulture, + no_xsoar_linter=no_xsoar_linter) + + # Run lint and test check on pack docker image + if self._facts["docker_engine"]: + self._run_lint_on_docker_image(no_pylint=no_pylint, + no_test=no_test, + no_pwsh_analyze=no_pwsh_analyze, + no_pwsh_test=no_pwsh_test, + keep_container=keep_container, + test_xml=test_xml) + except Exception as ex: + err = f'{self._pack_abs_dir}: Unexpected fatal exception: {str(ex)}' + logger.error(f"{err}. Traceback: {traceback.format_exc()}") + self._pkg_lint_status["errors"].append(err) + self._pkg_lint_status['exit_code'] += FAIL return self._pkg_lint_status def _gather_facts(self, modules: dict) -> bool: @@ -722,6 +727,11 @@ def _docker_run_pylint(self, test_image: str, keep_container: bool) -> Tuple[int container_obj.remove(force=True) except docker.errors.NotFound: pass + except requests.exceptions.ChunkedEncodingError as err: + # see: https://github.com/docker/docker-py/issues/2696#issuecomment-721322548 + if 'Connection broken: IncompleteRead' not in str(err): + raise + pass # Run container exit_code = SUCCESS From f5eabbeb99a89875ebf50981a85674b7abf8eff4 Mon Sep 17 00:00:00 2001 From: Guy Lichtman <1395797+glicht@users.noreply.github.com> Date: Thu, 26 Nov 2020 15:44:25 +0200 Subject: [PATCH 2/8] remove un-necessary pass --- demisto_sdk/commands/lint/linter.py | 1 - 1 file changed, 1 deletion(-) diff --git a/demisto_sdk/commands/lint/linter.py b/demisto_sdk/commands/lint/linter.py index 11e515c78ef..8034e0aa773 100644 --- a/demisto_sdk/commands/lint/linter.py +++ b/demisto_sdk/commands/lint/linter.py @@ -731,7 +731,6 @@ def _docker_run_pylint(self, test_image: str, keep_container: bool) -> Tuple[int # see: https://github.com/docker/docker-py/issues/2696#issuecomment-721322548 if 'Connection broken: IncompleteRead' not in str(err): raise - pass # Run container exit_code = SUCCESS From 46a22177ca40ca1368d268ceb1b75fb809592405 Mon Sep 17 00:00:00 2001 From: Guy Lichtman <1395797+glicht@users.noreply.github.com> Date: Thu, 26 Nov 2020 16:19:23 +0200 Subject: [PATCH 3/8] handle also other cases that we use containers --- demisto_sdk/commands/lint/linter.py | 76 +++++++++++++---------------- 1 file changed, 34 insertions(+), 42 deletions(-) diff --git a/demisto_sdk/commands/lint/linter.py b/demisto_sdk/commands/lint/linter.py index 8034e0aa773..23bd323ea79 100644 --- a/demisto_sdk/commands/lint/linter.py +++ b/demisto_sdk/commands/lint/linter.py @@ -706,6 +706,17 @@ def _docker_image_create(self, docker_base_image: List[Any]) -> Tuple[str, str]: return test_image_name, errors + def _docker_remove_container(self, container_name: str): + try: + container_obj = self._docker_client.containers.get(container_name) + container_obj.remove(force=True) + except docker.errors.NotFound: + pass + except requests.exceptions.ChunkedEncodingError as err: + # see: https://github.com/docker/docker-py/issues/2696#issuecomment-721322548 + if 'Connection broken: IncompleteRead' not in str(err): + raise + def _docker_run_pylint(self, test_image: str, keep_container: bool) -> Tuple[int, str]: """ Run Pylint in created test image @@ -721,28 +732,19 @@ def _docker_run_pylint(self, test_image: str, keep_container: bool) -> Tuple[int logger.info(f"{log_prompt} - Start") container_name = f"{self._pack_name}-pylint" # Check if previous run left container a live if it do, we remove it - container_obj: docker.models.containers.Container - try: - container_obj = self._docker_client.containers.get(container_name) - container_obj.remove(force=True) - except docker.errors.NotFound: - pass - except requests.exceptions.ChunkedEncodingError as err: - # see: https://github.com/docker/docker-py/issues/2696#issuecomment-721322548 - if 'Connection broken: IncompleteRead' not in str(err): - raise + self._docker_remove_container(container_name) # Run container exit_code = SUCCESS output = "" try: - container_obj = self._docker_client.containers.run(name=container_name, - image=test_image, - command=[ - build_pylint_command(self._facts["lint_files"])], - user=f"{os.getuid()}:4000", - detach=True, - environment=self._facts["env_vars"]) + container_obj: docker.models.containers.Container = self._docker_client.containers.run(name=container_name, + image=test_image, + command=[ + build_pylint_command(self._facts["lint_files"])], + user=f"{os.getuid()}:4000", + detach=True, + environment=self._facts["env_vars"]) stream_docker_container_output(container_obj.logs(stream=True)) # wait for container to finish container_status = container_obj.wait(condition="exited") @@ -800,25 +802,20 @@ def _docker_run_pytest(self, test_image: str, keep_container: bool, test_xml: st logger.info(f"{log_prompt} - Start") container_name = f"{self._pack_name}-pytest" # Check if previous run left container a live if it does, Remove it - container_obj: docker.models.containers.Container - try: - container_obj = self._docker_client.containers.get(container_name) - container_obj.remove(force=True) - except docker.errors.NotFound: - pass + self._docker_remove_container(container_name) # Collect tests exit_code = SUCCESS output = '' test_json = {} try: # Running pytest container - container_obj = self._docker_client.containers.run(name=container_name, - image=test_image, - command=[ - build_pytest_command(test_xml=test_xml, json=True)], - user=f"{os.getuid()}:4000", - detach=True, - environment=self._facts["env_vars"]) + container_obj: docker.models.containers.Container = self._docker_client.containers.run(name=container_name, + image=test_image, + command=[ + build_pytest_command(test_xml=test_xml, json=True)], + user=f"{os.getuid()}:4000", + detach=True, + environment=self._facts["env_vars"]) stream_docker_container_output(container_obj.logs(stream=True)) # Waiting for container to be finished container_status: dict = container_obj.wait(condition="exited") @@ -959,23 +956,18 @@ def _docker_run_pwsh_test(self, test_image: str, keep_container: bool) -> Tuple[ logger.info(f"{log_prompt} - Start") container_name = f"{self._pack_name}-pwsh-test" # Check if previous run left container a live if it do, we remove it - container_obj: docker.models.containers.Container - try: - container_obj = self._docker_client.containers.get(container_name) - container_obj.remove(force=True) - except docker.errors.NotFound: - pass + self._docker_remove_container(container_name) # Run container exit_code = SUCCESS output = "" try: - container_obj = self._docker_client.containers.run(name=container_name, - image=test_image, - command=build_pwsh_test_command(), - user=f"{os.getuid()}:4000", - detach=True, - environment=self._facts["env_vars"]) + container_obj: docker.models.containers.Container = self._docker_client.containers.run(name=container_name, + image=test_image, + command=build_pwsh_test_command(), + user=f"{os.getuid()}:4000", + detach=True, + environment=self._facts["env_vars"]) stream_docker_container_output(container_obj.logs(stream=True)) # wait for container to finish container_status = container_obj.wait(condition="exited") From 8756273c0dd75fc7366a61654243360d16589a3f Mon Sep 17 00:00:00 2001 From: Guy Lichtman <1395797+glicht@users.noreply.github.com> Date: Thu, 26 Nov 2020 16:32:48 +0200 Subject: [PATCH 4/8] fix exception string --- demisto_sdk/commands/lint/linter.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/demisto_sdk/commands/lint/linter.py b/demisto_sdk/commands/lint/linter.py index 23bd323ea79..bf7b4eebbec 100644 --- a/demisto_sdk/commands/lint/linter.py +++ b/demisto_sdk/commands/lint/linter.py @@ -714,7 +714,7 @@ def _docker_remove_container(self, container_name: str): pass except requests.exceptions.ChunkedEncodingError as err: # see: https://github.com/docker/docker-py/issues/2696#issuecomment-721322548 - if 'Connection broken: IncompleteRead' not in str(err): + if 'Connection broken' not in str(err): raise def _docker_run_pylint(self, test_image: str, keep_container: bool) -> Tuple[int, str]: From 8c01037d4e19c27dbbfa95fdec200155130bcc6a Mon Sep 17 00:00:00 2001 From: Guy Lichtman <1395797+glicht@users.noreply.github.com> Date: Thu, 26 Nov 2020 18:17:07 +0200 Subject: [PATCH 5/8] improve no-docker mode --- demisto_sdk/commands/lint/helpers.py | 4 ++-- demisto_sdk/commands/lint/linter.py | 17 +++++++++++------ 2 files changed, 13 insertions(+), 8 deletions(-) diff --git a/demisto_sdk/commands/lint/helpers.py b/demisto_sdk/commands/lint/helpers.py index f8a5a5a1614..8464a93f959 100644 --- a/demisto_sdk/commands/lint/helpers.py +++ b/demisto_sdk/commands/lint/helpers.py @@ -95,9 +95,9 @@ def build_skipped_exit_code(no_flake8: bool, no_bandit: bool, no_mypy: bool, no_ skipped_code |= EXIT_CODES["XSOAR_linter"] if no_bandit: skipped_code |= EXIT_CODES["bandit"] - if no_mypy or not docker_engine: + if no_mypy: skipped_code |= EXIT_CODES["mypy"] - if no_vulture or not docker_engine: + if no_vulture: skipped_code |= EXIT_CODES["vulture"] if no_pylint or not docker_engine: skipped_code |= EXIT_CODES["pylint"] diff --git a/demisto_sdk/commands/lint/linter.py b/demisto_sdk/commands/lint/linter.py index bf7b4eebbec..af43d171eac 100644 --- a/demisto_sdk/commands/lint/linter.py +++ b/demisto_sdk/commands/lint/linter.py @@ -234,6 +234,11 @@ def _gather_facts(self, modules: dict) -> bool: logger.info(f"{log_prompt} - Additional package Pypi packages found - {additional_req}") except (FileNotFoundError, IOError): self._pkg_lint_status["errors"].append('Unable to parse test-requirements.txt in package') + elif not self._facts["python_version"]: + # get python version from yml + pynum = 3.7 if (script_obj.get('subtype', 'python3') == 'python3') else 2.7 + self._facts["python_version"] = pynum + logger.info(f"{log_prompt} - Using python version from yml: {pynum}") # Get lint files lint_files = set(self._pack_abs_dir.glob(["*.py", "!__init__.py", "!*.tmp"], flags=NEGATE)) @@ -294,17 +299,17 @@ def _run_lint_in_host(self, no_flake8: bool, no_bandit: bool, no_mypy: bool, no_ exit_code = SUCCESS output = "" if lint_check == "flake8" and not no_flake8: - exit_code, output = self._run_flake8(py_num=self._facts["images"][0][1], + exit_code, output = self._run_flake8(py_num=self._facts["python_version"], lint_files=self._facts["lint_files"]) elif lint_check == "XSOAR_linter" and not no_xsoar_linter: - exit_code, output = self._run_xsoar_linter(py_num=self._facts["images"][0][1], + exit_code, output = self._run_xsoar_linter(py_num=self._facts["python_version"], lint_files=self._facts["lint_files"]) elif lint_check == "bandit" and not no_bandit: exit_code, output = self._run_bandit(lint_files=self._facts["lint_files"]) - elif lint_check == "mypy" and not no_mypy and self._facts["docker_engine"]: - exit_code, output = self._run_mypy(py_num=self._facts["images"][0][1], + elif lint_check == "mypy" and not no_mypy: + exit_code, output = self._run_mypy(py_num=self._facts["python_version"], lint_files=self._facts["lint_files"]) - elif lint_check == "vulture" and not no_vulture and self._facts["docker_engine"]: + elif lint_check == "vulture" and not no_vulture: exit_code, output = self._run_vulture(py_num=self._facts["python_version"], lint_files=self._facts["lint_files"]) @@ -328,7 +333,7 @@ def _run_lint_in_host(self, no_flake8: bool, no_bandit: bool, no_mypy: bool, no_ exit_code = SUCCESS output = "" if lint_check == "flake8" and not no_flake8: - exit_code, output = self._run_flake8(py_num=self._facts["images"][0][1], + exit_code, output = self._run_flake8(py_num=self._facts["python_version"], lint_files=self._facts["lint_unittest_files"]) if exit_code: error, warning, other = split_warnings_errors(output) From 3baf9ae1d7975bb2760bf75b607bbb386ca0ee89 Mon Sep 17 00:00:00 2001 From: Guy Lichtman <1395797+glicht@users.noreply.github.com> Date: Thu, 26 Nov 2020 18:31:12 +0200 Subject: [PATCH 6/8] fix tests --- demisto_sdk/commands/lint/linter.py | 3 ++- .../commands/lint/tests/test_linter/os_runner_test.py | 10 ++++++---- 2 files changed, 8 insertions(+), 5 deletions(-) diff --git a/demisto_sdk/commands/lint/linter.py b/demisto_sdk/commands/lint/linter.py index af43d171eac..aa934f53668 100644 --- a/demisto_sdk/commands/lint/linter.py +++ b/demisto_sdk/commands/lint/linter.py @@ -4,6 +4,7 @@ import json import logging import os +import platform import traceback from copy import deepcopy from typing import Any, Dict, List, Optional, Tuple @@ -719,7 +720,7 @@ def _docker_remove_container(self, container_name: str): pass except requests.exceptions.ChunkedEncodingError as err: # see: https://github.com/docker/docker-py/issues/2696#issuecomment-721322548 - if 'Connection broken' not in str(err): + if platform.system() != 'Darwin' or 'Connection broken' not in str(err): raise def _docker_run_pylint(self, test_image: str, keep_container: bool) -> Tuple[int, str]: diff --git a/demisto_sdk/commands/lint/tests/test_linter/os_runner_test.py b/demisto_sdk/commands/lint/tests/test_linter/os_runner_test.py index 1d42a85be0d..08d6355bdb5 100644 --- a/demisto_sdk/commands/lint/tests/test_linter/os_runner_test.py +++ b/demisto_sdk/commands/lint/tests/test_linter/os_runner_test.py @@ -339,12 +339,13 @@ def test_fail_lint_on_only_test_file(self, mocker, linter_obj, lint_files): from demisto_sdk.commands.lint.linter import EXIT_CODES unittest_path = lint_files[0].parent / 'intergration_sample_test.py' mocker.patch.dict(linter_obj._facts, { - "images": [["image", "3.7"]], + "images": [["image", 3.7]], "test": False, "version_two": False, "lint_files": [], "lint_unittest_files": [unittest_path], - "additional_requirements": [] + "additional_requirements": [], + "python_version": 3.7, }) mocker.patch.object(linter_obj, '_run_flake8') linter_obj._run_flake8.return_value = (0b1, 'Error') @@ -390,12 +391,13 @@ def test_fail_lint_on_normal_and_test_file(self, mocker, linter_obj, lint_files) from demisto_sdk.commands.lint.linter import EXIT_CODES unittest_path = lint_files[0].parent / 'intergration_sample_test.py' mocker.patch.dict(linter_obj._facts, { - "images": [["image", "3.7"]], + "images": [["image", 3.7]], "test": False, "version_two": False, "lint_files": lint_files, "lint_unittest_files": [unittest_path], - "additional_requirements": [] + "additional_requirements": [], + "python_version": 3.7, }) mocker.patch.object(linter_obj, '_run_flake8') linter_obj._run_flake8.return_value = (0b1, 'Error') From 8b5b33af32f1ba0d1f5984c84ce8841fe25f8db3 Mon Sep 17 00:00:00 2001 From: Guy Lichtman <1395797+glicht@users.noreply.github.com> Date: Fri, 27 Nov 2020 10:27:23 +0200 Subject: [PATCH 7/8] fail if we are in CI content build and there is no docker --- demisto_sdk/commands/lint/lint_manager.py | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/demisto_sdk/commands/lint/lint_manager.py b/demisto_sdk/commands/lint/lint_manager.py index 110df7844a8..4f15e5e9a59 100644 --- a/demisto_sdk/commands/lint/lint_manager.py +++ b/demisto_sdk/commands/lint/lint_manager.py @@ -133,7 +133,10 @@ def _gather_facts() -> Dict[str, Any]: docker_client: docker.DockerClient = docker.from_env() try: docker_client.ping() - except (requests.exceptions.ConnectionError, urllib3.exceptions.ProtocolError, docker.errors.APIError): + except (requests.exceptions.ConnectionError, urllib3.exceptions.ProtocolError, docker.errors.APIError) as ex: + if os.getenv("CI") and os.getenv("CIRCLE_PROJECT_REPONAME") == "content": + # when running lint in content we fail if docker isn't available for some reason + raise ValueError("Docker engine not available and we are in content CI env. Can not run lint!!") from ex facts["docker_engine"] = False print_warning("Can't communicate with Docker daemon - check your docker Engine is ON - Skipping lint, " "test which require docker!") From 2d9bb4fe1e25d0a44f457125a6fb078f237f49fb Mon Sep 17 00:00:00 2001 From: Guy Lichtman <1395797+glicht@users.noreply.github.com> Date: Sat, 28 Nov 2020 23:40:14 +0200 Subject: [PATCH 8/8] update release notes --- CHANGELOG.md | 2 ++ 1 file changed, 2 insertions(+) diff --git a/CHANGELOG.md b/CHANGELOG.md index 7b54d72f58a..3499d7073b9 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -1,6 +1,8 @@ # Changelog * Fixed an issue where the ***generate-docs*** command reset the enumeration of line numbering after an MD table. * Updated the **upload** command to support mappers. +* Fixed an issue with support for Docker Desktop on Mac version 2.5.0+. +* Added support for vulture and mypy linting when running without docker. # 1.2.10 * Added validation for approved content pack use-cases and tags.