Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

fail on unexpected exception + handle mac os docker #904

Merged
merged 9 commits into from
Nov 30, 2020
Merged

fail on unexpected exception + handle mac os docker #904

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
9 commits
Select commit Hold shift + click to select a range
7dc8fa7
fail on unexpected exception + handle mac os docker
glicht Nov 26, 2020
f5eabbe
remove un-necessary pass
glicht Nov 26, 2020
46a2217
handle also other cases that we use containers
glicht Nov 26, 2020
8756273
fix exception string
glicht Nov 26, 2020
8c01037
improve no-docker mode
glicht Nov 26, 2020
3baf9ae
fix tests
glicht Nov 26, 2020
8b5b33a
fail if we are in CI content build and there is no docker
glicht Nov 27, 2020
2d9bb4f
update release notes
glicht Nov 28, 2020
51158d4
Merge remote-tracking branch 'origin/master' into linter-fix-macos
glicht Nov 30, 2020
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 2 additions & 0 deletions CHANGELOG.md
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -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.
* 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
Open in desktop
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
Open in desktop
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
Open in desktop
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
Open in desktop
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