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.

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

use precommit template from content #3159

Merged
merged 24 commits into from
Jun 19, 2023
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
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
3 changes: 2 additions & 1 deletion CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -11,7 +11,8 @@
* Fixed an issue where **validate** always returned *XSIAM Dashboards* and *Correlation Rules* files as valid.
* Added `GR107` validation to **validate** using the graph validations to check that no deprecated items are used by non-deprecated content.
* Fixed an issue where the **modeling-rules test** command failed to get the existence of dataset in cases where the dataset takes more than 1 minute to get indexed.
* Fixed an issue where an internal method that caused warning messages when reading md files.
* Moved the **pre-commmit** command template to the `demisto/content` repository, where it's easier to maintain.
* Fixed an issue where an internal method caused warning messages when reading md files.

## 1.16.0
* Added a check to **is_docker_image_latest_tag** to only fail the validation on non-latest image tag when the current tag is older than 3 days.
Expand Down
36 changes: 33 additions & 3 deletions demisto_sdk/commands/common/tools.py
Original file line number Diff line number Diff line change
Expand Up @@ -830,6 +830,34 @@ def get_file(
return {}


def get_file_or_remote(file_path: Path, clear_cache=False):
content_path = get_content_path()
relative_file_path = None
if file_path.is_absolute():
absolute_file_path = file_path
try:
relative_file_path = file_path.relative_to(content_path)
except ValueError:
logger.debug(
f"{file_path} is not a subpath of {content_path}. If the file does not exists locally, it could not be fetched."
)
else:
absolute_file_path = content_path / file_path
relative_file_path = file_path
try:
return get_file(absolute_file_path, clear_cache=clear_cache)
except FileNotFoundError:
logger.warning(
f"Could not read/find {absolute_file_path} locally, fetching from remote"
)
if not relative_file_path:
logger.error(
f"The file path provided {file_path} is not a subpath of {content_path}. could not fetch from remote."
)
raise
return get_remote_file(relative_file_path)
ilaner marked this conversation as resolved.
Show resolved Hide resolved


def get_yaml(file_path, cache_clear=False):
if cache_clear:
get_file.cache_clear()
Expand Down Expand Up @@ -1988,7 +2016,7 @@ def get_latest_upload_flow_commit_hash() -> str:
return last_commit


def get_content_path() -> Union[str, PathLike, None]:
def get_content_path() -> Path:
"""Get abs content path, from any CWD
Returns:
str: Absolute content path
Expand All @@ -2006,13 +2034,15 @@ def get_content_path() -> Union[str, PathLike, None]:

if not is_fork_repo and not is_external_repo:
raise git.InvalidGitRepositoryError
return git_repo.working_dir
if not git_repo.working_dir:
return Path.cwd()
return Path(git_repo.working_dir)
except (git.InvalidGitRepositoryError, git.NoSuchPathError):
if not os.getenv("DEMISTO_SDK_IGNORE_CONTENT_WARNING"):
logger.info(
"[yellow]Please run demisto-sdk in content repository![/yellow]"
)
return ""
return Path(".")


def run_command_os(
Expand Down
3 changes: 1 addition & 2 deletions demisto_sdk/commands/content_graph/neo4j_service.py
Original file line number Diff line number Diff line change
Expand Up @@ -15,8 +15,7 @@
NEO4J_PASSWORD,
)

REPO_PATH = CONTENT_PATH

REPO_PATH = CONTENT_PATH.absolute()
NEO4J_VERSION = "5.5.0"

NEO4J_SERVICE_IMAGE = f"neo4j:{NEO4J_VERSION}"
Expand Down
30 changes: 20 additions & 10 deletions demisto_sdk/commands/pre_commit/pre_commit_command.py
Original file line number Diff line number Diff line change
Expand Up @@ -22,7 +22,11 @@
from demisto_sdk.commands.common.git_util import GitUtil
from demisto_sdk.commands.common.handlers import JSON_Handler, YAML_Handler
from demisto_sdk.commands.common.logger import logger
from demisto_sdk.commands.common.tools import get_last_remote_release_version
from demisto_sdk.commands.common.tools import (
get_file_or_remote,
get_last_remote_release_version,
str2bool,
)
from demisto_sdk.commands.content_graph.objects.base_content import BaseContent
from demisto_sdk.commands.content_graph.objects.integration_script import (
IntegrationScript,
Expand All @@ -36,13 +40,13 @@
yaml = YAML_Handler()
json = JSON_Handler()

IS_GITHUB_ACTIONS = os.getenv("GITHUB_ACTIONS", False)
IS_GITHUB_ACTIONS = str2bool(os.getenv("GITHUB_ACTIONS"))

PRECOMMIT_TEMPLATE_PATH = Path(__file__).parent / ".pre-commit-config_template.yaml"
PRECOMMIT_TEMPLATE_PATH = CONTENT_PATH / ".pre-commit-config_template.yaml"
PRECOMMIT_PATH = CONTENT_PATH / ".pre-commit-config-content.yaml"

# UNSKIP no-implicit-optional once mypy is updated
SKIPPED_HOOKS = {"format", "validate", "secrets", "no-implicit-optional"}
SKIPPED_HOOKS = {"format", "validate", "secrets"}

INTEGRATION_SCRIPT_REGEX = re.compile(r"^Packs/.*/(?:Integrations|Scripts)/.*.yml$")

Expand All @@ -62,14 +66,17 @@ def __post_init__(self):
self.all_files = set(
itertools.chain.from_iterable(self.python_version_to_files.values())
)
with open(PRECOMMIT_TEMPLATE_PATH) as f:
self.precommit_template = yaml.load(f)

self.precommit_template = get_file_or_remote(PRECOMMIT_TEMPLATE_PATH)
if not isinstance(self.precommit_template, dict):
raise TypeError(
f"Pre-commit template in {PRECOMMIT_TEMPLATE_PATH} is not a dictionary."
)
# changes the demisto-sdk revision to the latest release version (or the debug commit hash)
# to debug, modify the DEMISTO_SDK_COMMIT_HASH_DEBUG variable to your demisto-sdk commit hash
self._get_repos(self.precommit_template)[
"https://github.com/demisto/demisto-sdk"
]["rev"] = self.demisto_sdk_commit_hash
self.hooks = self._get_hooks(self.precommit_template)

@staticmethod
def _get_repos(pre_commit_config: dict) -> dict:
Expand All @@ -84,14 +91,16 @@ def _get_hooks(pre_commit_config: dict) -> dict:
for repo in pre_commit_config["repos"]:
for hook in repo["hooks"]:
hooks[hook["id"]] = hook
# if the hook has a skip key, we add it to the SKIPPED_HOOKS set
if hook.pop("skip", None):
dorschw marked this conversation as resolved.
Show resolved Hide resolved
SKIPPED_HOOKS.add(hook["id"])
return hooks

def prepare_hooks(
self,
pre_commit_config: dict,
hooks: dict,
python_version: str,
) -> None:
hooks = self._get_hooks(pre_commit_config)
PyclnHook(hooks["pycln"]).prepare_hook(PYTHONPATH)
RuffHook(hooks["ruff"]).prepare_hook(python_version, IS_GITHUB_ACTIONS)
MypyHook(hooks["mypy"]).prepare_hook(python_version)
Expand Down Expand Up @@ -130,6 +139,7 @@ def run(
precommit_env["DEMISTO_SDK_CONTENT_PATH"] = str(CONTENT_PATH)
for python_version, changed_files in self.python_version_to_files.items():
precommit_config = deepcopy(self.precommit_template)
assert isinstance(precommit_config, dict)
changed_files_string = ", ".join(
sorted((str(changed_path) for changed_path in changed_files))
)
Expand Down Expand Up @@ -157,7 +167,7 @@ def run(
if response.returncode:
ret_val = response.returncode
continue
self.prepare_hooks(precommit_config, python_version)
self.prepare_hooks(self._get_hooks(precommit_config), python_version)
with open(PRECOMMIT_PATH, "w") as f:
yaml.dump(precommit_config, f)
# use chunks because OS does not support such large comments
Expand Down
5 changes: 5 additions & 0 deletions demisto_sdk/commands/pre_commit/tests/pre_commit_test.py
Original file line number Diff line number Diff line change
Expand Up @@ -36,6 +36,11 @@ def test_config_files(mocker, repo: Repo, is_test: bool):
Then:
Categorize the scripts and integration by python version, and make sure that pre-commit configuration is created for each
"""
mocker.patch.object(
pre_commit_command,
"PRECOMMIT_TEMPLATE_PATH",
TEST_DATA_PATH / ".pre-commit-config_template.yaml",
)
pack1 = repo.create_pack("Pack1")
mocker.patch.object(pre_commit_command, "CONTENT_PATH", Path(repo.path))

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -58,6 +58,7 @@ repos:
# `--python-version` argument is replaced in runtime
--python-version=3.10,
]
exclude: "test_data|tests_data|.venv|.*_test.py$|infrastructure_tests"

# enable with --test
- id: run-unit-tests
Expand All @@ -73,3 +74,4 @@ repos:
args: ["--ignore-entropy"]

- id: no-implicit-optional
skip: true
10 changes: 5 additions & 5 deletions demisto_sdk/tests/integration_tests/validate_integration_test.py
Original file line number Diff line number Diff line change
Expand Up @@ -4899,8 +4899,8 @@ def test_passing_validation_using_git(self, mocker, repo):
"Running validation on modified files",
"Running validation on newly added files",
"Running validation on changed pack unique files",
"Validating Packs/PackName1 unique pack files",
"Validating Packs/PackName2 unique pack files",
"Validating ./Packs/PackName1 unique pack files",
"Validating ./Packs/PackName2 unique pack files",
f"Validating {integration.yml.rel_path} as integration",
f"Validating {incident_field.get_path_from_pack()} as incidentfield",
f"Validating {dashboard.get_path_from_pack()} as dashboard",
Expand Down Expand Up @@ -4991,8 +4991,8 @@ def test_failing_validation_using_git(self, mocker, repo):
"Running validation on modified files",
"Running validation on newly added files",
"Running validation on changed pack unique files",
"Validating Packs/PackName1 unique pack files",
"Validating Packs/PackName2 unique pack files",
"Validating ./Packs/PackName1 unique pack files",
"Validating ./Packs/PackName2 unique pack files",
f"Validating {integration.yml.rel_path} as integration",
f"Validating {incident_field.get_path_from_pack()} as incidentfield",
f"Validating {dashboard.get_path_from_pack()} as dashboard",
Expand Down Expand Up @@ -5074,7 +5074,7 @@ def test_validation_using_git_without_pack_dependencies(self, mocker, repo):
"Running validation on modified files",
"Running validation on newly added files",
"Running validation on changed pack unique files",
"Validating Packs/FeedAzure unique pack files",
"Validating ./Packs/FeedAzure unique pack files",
]
]
)
Expand Down