Skip to content

Commit

Permalink
fail on unexpected exception + handle mac os docker (#904)
Browse files Browse the repository at this point in the history 
* fail on unexpected exception + handle mac os docker

* remove un-necessary pass

* handle also other cases that we use containers

* fix exception string

* improve no-docker mode

* fix tests

* fail if we are in CI content build and there is no docker

* update release notes
  • Loading branch information
@glicht
glicht authored Nov 30, 2020
1 parent 241cb17 commit 869e11f
Show file tree
Hide file tree
Showing 5 changed files with 91 additions and 76 deletions.
2 changes: 2 additions & 0 deletions CHANGELOG.md
View file
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,8 @@
# 1.2.11
* 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.
* Fixed an issue where exceptions were no printed in the **format** while the *--verbose* flag is set.
* Fixed an issue where *--assume-yes* flag did not work in the **format** command when running on a playbook without a `fromversion` field.
* Fixed an issue where the **format** command would fail in case `conf.json` file was not found instead of skipping the update.
Expand Down
7 changes: 4 additions & 3 deletions demisto_sdk/commands/lint/helpers.py
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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"]
Expand Down Expand Up @@ -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
Expand Down
5 changes: 4 additions & 1 deletion demisto_sdk/commands/lint/lint_manager.py
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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!")
Expand Down
143 changes: 75 additions & 68 deletions demisto_sdk/commands/lint/linter.py
View file
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,8 @@
import json
import logging
import os
import platform
import traceback
from copy import deepcopy
from typing import Any, Dict, List, Optional, Tuple

Expand Down Expand Up @@ -123,30 +125,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:
Expand Down Expand Up @@ -229,6 +235,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))
Expand Down Expand Up @@ -289,17 +300,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"])

Expand All @@ -323,7 +334,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)
Expand Down Expand Up @@ -701,6 +712,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 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]:
""" Run Pylint in created test image
Expand All @@ -716,24 +738,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
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")
Expand Down Expand Up @@ -791,25 +808,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")
Expand Down Expand Up @@ -950,23 +962,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")
Expand Down
10 changes: 6 additions & 4 deletions demisto_sdk/commands/lint/tests/test_linter/os_runner_test.py
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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')
Expand Down Expand Up @@ -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')
Expand Down

0 comments on commit 869e11f

Please sign in to comment.