Resolve Circular Imports (#3912)

* Remove `tools` module imports from `logger` * Fix var name * Update log files location * Add changelog * Update help message * Apply suggestions from code review Co-authored-by: Guy Afik <[email protected]> * black * Add tests, and update logger functions * Update default logs directory * Update pre-commit to allow line-breaks (represented by 2 spaces) on markdown files * Update `log-file-path` for commands that use `typer` * Minor improvements * Update CircliCI logs path parameter * Apply suggestions from code review Co-authored-by: tkatzir <[email protected]> * Minor improvements * pre-commit fix * Use logger import instead of `getLogger` * Move environment variable names to constants module * Resolve redundant loggers setup issue * Add integration tests * Fix & update logging flags for typer commands * Add new-format changelogs * Add changelogs * Utilize monkeypatch * Add given-when-then * Remove unused var * Remove usage of caplog * pre-commit * Minor improvements * Fix release-notes tag --------- Co-authored-by: Guy Afik <[email protected]> Co-authored-by: tkatzir <[email protected]>
demisto · Jan 17, 2024 · c73d3d1 · c73d3d1
1 parent 07bd37d
commit c73d3d1
Show file tree

Hide file tree

Showing 41 changed files with 614 additions and 281 deletions.
diff --git a/.changelog/3912.yml b/.changelog/3912.yml
@@ -0,0 +1,4 @@
+changes:
+- description: Log files will now be saved by default to `$HOME/.demisto-sdk/logs`. This behavior can be overridden by the `--log-file-path` flag, or the `DEMISTO_SDK_LOG_FILE_PATH` environment variable.
+  type: feature
+pr_number: 3912
diff --git a/.circleci/config.yml b/.circleci/config.yml
@@ -314,7 +314,7 @@ jobs:
               source $(poetry env info --path)/bin/activate
               export ARTIFACTS_FOLDER="/home/circleci/project/artifacts"
               mkdir -p $ARTIFACTS_FOLDER
-              export DEMISTO_SDK_LOG_FILE_PATH=$ARTIFACTS_FOLDER/demisto_sdk_debug.log
+              export DEMISTO_SDK_LOG_FILE_PATH=$ARTIFACTS_FOLDER
               cd content
               demisto-sdk -v
               echo $(echo '{"node_version": "'$(node --version)'","npm_list":'$(npm list --json)'}')
@@ -335,7 +335,7 @@ jobs:
               source $(poetry env info --path)/bin/activate
               export ARTIFACTS_FOLDER="/home/circleci/project/artifacts"
               mkdir -p $ARTIFACTS_FOLDER
-              export DEMISTO_SDK_LOG_FILE_PATH=$ARTIFACTS_FOLDER/demisto_sdk_debug.log
+              export DEMISTO_SDK_LOG_FILE_PATH=$ARTIFACTS_FOLDER
               cd content
               demisto-sdk -v
               echo $(echo '{"node_version": "'$(node --version)'","npm_list":'$(npm list --json)'}')
@@ -436,7 +436,7 @@ jobs:
             source $(poetry env info --path)/bin/activate
             export ARTIFACTS_FOLDER="/home/circleci/project/artifacts"
             mkdir -p $ARTIFACTS_FOLDER
-            export DEMISTO_SDK_LOG_FILE_PATH=$ARTIFACTS_FOLDER/demisto_sdk_debug.log
+            export DEMISTO_SDK_LOG_FILE_PATH=$ARTIFACTS_FOLDER
             cd content
             demisto-sdk -v
             mkdir ./tmp

diff --git a/.pre-commit-config.yaml b/.pre-commit-config.yaml
@@ -204,6 +204,8 @@ repos:
   rev: v4.4.0
   hooks:
   - id: trailing-whitespace
+    args:
+    - --markdown-linebreak-ext=md
   - id: end-of-file-fixer
   - id: check-docstring-first
     exclude: demisto_sdk/commands/init/templates/.*

diff --git a/README.md b/README.md
@@ -120,9 +120,20 @@ Supported commands:
 20. [generate-yml-from-python](https://xsoar.pan.dev/docs/integrations/yml-from-python-code-gen)
 21. [generate-unit-tests](https://github.com/demisto/demisto-sdk/blob/master/demisto_sdk/commands/generate_unit_tests/README.md)
 22. [pre-commit (experimental)](https://github.com/demisto/demisto-sdk/blob/master/demisto_sdk/commands/pre_commit/README.md)
----
 23. [setup-env](https://github.com/demisto/demisto-sdk/blob/master/demisto_sdk/commands/setup_env/README.md)
 
+---
+
+### Logs
+
+Log files are generated and stored automatically by default in the user's home directory:  
+**Linux / macOS**: `$HOME/.demisto-sdk/logs`  
+**Windows**: `%USERPROFILE%\.demisto-sdk\logs`  
+
+The default directory can be overriden using the `--log-file-path` flag, or the `DEMISTO_SDK_LOG_FILE_PATH` environment variable.
+
+---
+
 ### Customizable command configuration
 
 You can create your own configuration for the `demisto-sdk` commands by creating a file named `.demisto-sdk-conf` within the directory from which you run the commands.
@@ -167,7 +178,7 @@ MIT - See [LICENSE](LICENSE) for more information.
 
 ---
 
-## How to setup development environment?
+## How to setup a development environment?
 
 Follow the guide found [here](CONTRIBUTION.md#2-install-demisto-sdk-dev-environment) to setup your `demisto-sdk` dev environment.
 The development environment is connected to the branch you are currently using in the SDK repository.

diff --git a/Utils/github_workflow_scripts/changelog_validation_scripts/validate_changelog.py b/Utils/github_workflow_scripts/changelog_validation_scripts/validate_changelog.py
@@ -1,11 +1,9 @@
 import argparse
-import logging
 import sys
 
+from demisto_sdk.commands.common.logger import logger
 from demisto_sdk.scripts.changelog.changelog import Changelog
 
-logger = logging.getLogger("demisto-sdk")
-
 
 def validate_changelog_and_logs(pr_num: str, pr_name: str) -> bool:
     try:

diff --git a/demisto_sdk/__main__.py b/demisto_sdk/__main__.py
@@ -37,7 +37,11 @@
 from demisto_sdk.commands.common.cpu_count import cpu_count
 from demisto_sdk.commands.common.handlers import DEFAULT_JSON_HANDLER as json
 from demisto_sdk.commands.common.hook_validations.readme import ReadMeValidator
-from demisto_sdk.commands.common.logger import handle_deprecated_args, logging_setup
+from demisto_sdk.commands.common.logger import (
+    handle_deprecated_args,
+    logger,
+    logging_setup,
+)
 from demisto_sdk.commands.common.tools import (
     find_type,
     get_last_remote_release_version,
@@ -70,8 +74,6 @@
     "internet, un-set the DEMISTO_SDK_OFFLINE_ENV environment variable.[/red]"
 )
 
-logger = logging.getLogger("demisto-sdk")
-
 
 # Third party packages
 
@@ -158,16 +160,13 @@ def get_context_arg(args):
         help="Minimum logging threshold for the file logger."
         " Possible values: DEBUG, INFO, WARNING, ERROR.",
     )
-    @click.option(
-        "--log-file-path",
-        help="Path to the log file. Default: Content root path.",
-    )
+    @click.option("--log-file-path", help="Path to save log files onto.")
     @functools.wraps(func)
     def wrapper(*args, **kwargs):
         logging_setup(
             console_log_threshold=kwargs.get("console_log_threshold") or logging.INFO,
             file_log_threshold=kwargs.get("file_log_threshold") or logging.DEBUG,
-            log_file_path=kwargs.get("log_file_path") or None,
+            log_file_path=kwargs.get("log_file_path"),
         )
 
         handle_deprecated_args(get_context_arg(args).args)
@@ -205,9 +204,8 @@ def main(ctx, config, version, release_notes, **kwargs):
         console_log_threshold=kwargs.get("console_log_threshold", logging.INFO),
         file_log_threshold=kwargs.get("file_log_threshold", logging.DEBUG),
         log_file_path=kwargs.get("log_file_path"),
+        skip_log_file_creation=True,  # Log file creation is handled in the logger setup of the sub-command
     )
-    global logger
-    logger = logging.getLogger("demisto-sdk")
     handle_deprecated_args(ctx.args)
 
     config.configuration = Configuration()
@@ -1272,12 +1270,8 @@ def lint(ctx, **kwargs):
     type=str,
 )
 @click.pass_context
+@logging_setup_decorator
 def coverage_analyze(ctx, **kwargs):
-    logger = logging_setup(
-        console_log_threshold=kwargs.get("console_log_threshold") or logging.INFO,
-        file_log_threshold=kwargs.get("file_log_threshold") or logging.DEBUG,
-        log_file_path=kwargs.get("log_file_path") or None,
-    )
     from demisto_sdk.commands.coverage_analyze.coverage_report import CoverageReport
 
     try:
@@ -2684,6 +2678,7 @@ def find_dependencies(ctx, **kwargs):
 )
 @pass_config
 @click.pass_context
+@logging_setup_decorator
 def postman_codegen(
     ctx,
     config,
@@ -2697,11 +2692,6 @@ def postman_codegen(
     **kwargs,
 ):
     """Generates a Cortex XSOAR integration given a Postman collection 2.1 JSON file."""
-    logger = logging_setup(
-        console_log_threshold=kwargs.get("console_log_threshold") or logging.INFO,
-        file_log_threshold=kwargs.get("file_log_threshold") or logging.DEBUG,
-        log_file_path=kwargs.get("log_file_path") or None,
-    )
     from demisto_sdk.commands.postman_codegen.postman_codegen import (
         postman_to_autogen_configuration,
     )
@@ -3584,7 +3574,7 @@ def pre_commit(
         dir_okay=True,
         resolve_path=True,
         show_default=False,
-        help=("The paths to run pre-commit on. May pass multiple paths."),
+        help="The paths to run pre-commit on. May pass multiple paths.",
     ),
     staged_only: bool = typer.Option(
         False, "--staged-only", help="Whether to run only on staged files"
@@ -3631,7 +3621,28 @@ def pre_commit(
         True, "--docker/--no-docker", help="Whether to run docker based hooks or not."
     ),
     run_hook: Optional[str] = typer.Argument(None, help="A specific hook to run"),
+    console_log_threshold: str = typer.Option(
+        "INFO",
+        "--console-log-threshold",
+        help="Minimum logging threshold for the console logger.",
+    ),
+    file_log_threshold: str = typer.Option(
+        "DEBUG",
+        "--file-log-threshold",
+        help="Minimum logging threshold for the file logger.",
+    ),
+    log_file_path: Optional[str] = typer.Option(
+        None,
+        "--log-file-path",
+        help="Path to save log files onto.",
+    ),
 ):
+    logging_setup(
+        console_log_threshold=console_log_threshold,
+        file_log_threshold=file_log_threshold,
+        log_file_path=log_file_path,
+    )
+
     from demisto_sdk.commands.pre_commit.pre_commit_command import pre_commit_manager
 
     return_code = pre_commit_manager(

diff --git a/demisto_sdk/commands/common/constants.py b/demisto_sdk/commands/common/constants.py
@@ -2,10 +2,49 @@
 import re
 from enum import Enum
 from functools import reduce
+from pathlib import Path
 from typing import Dict, List
 
 from packaging.version import Version
 
+# Note: Do NOT add imports of internal modules here, as it may cause circular imports.
+
+
+PROJECT_DATA_DIR = Path.home() / ".demisto-sdk"
+LOGS_DIR = PROJECT_DATA_DIR / "logs"
+LOG_FILE_NAME = "demisto_sdk_debug.log"
+
+# --- Environment Variables ---
+# General
+ENV_DEMISTO_SDK_MARKETPLACE = "DEMISTO_SDK_MARKETPLACE"
+DEMISTO_GIT_PRIMARY_BRANCH = os.getenv("DEMISTO_DEFAULT_BRANCH", "master")
+DEMISTO_GIT_UPSTREAM = os.getenv("DEMISTO_DEFAULT_REMOTE", "origin")
+DEMISTO_SDK_CI_SERVER_HOST = os.getenv("CI_SERVER_HOST", "gitlab.xdr.pan.local")
+DEMISTO_SDK_OFFICIAL_CONTENT_PROJECT_ID = os.getenv(
+    "CI_PROJECT_ID", "1061"
+)  # Default value is the ID of the content repo on GitLab
+ENV_SDK_WORKING_OFFLINE = "DEMISTO_SDK_OFFLINE_ENV"
+
+
+# Authentication
+DEMISTO_BASE_URL = "DEMISTO_BASE_URL"
+DEMISTO_USERNAME = "DEMISTO_USERNAME"
+DEMISTO_PASSWORD = "DEMISTO_PASSWORD"  # guardrails-disable-line
+DEMISTO_KEY = "DEMISTO_API_KEY"
+AUTH_ID = "XSIAM_AUTH_ID"
+XSIAM_TOKEN = "XSIAM_TOKEN"
+XSIAM_COLLECTOR_TOKEN = "XSIAM_COLLECTOR_TOKEN"
+DEMISTO_VERIFY_SSL = "DEMISTO_VERIFY_SSL"
+
+# Logging
+DEMISTO_SDK_LOG_FILE_PATH = "DEMISTO_SDK_LOG_FILE_PATH"
+DEMISTO_SDK_LOG_NOTIFY_PATH = "DEMISTO_SDK_LOG_NOTIFY_PATH"
+DEMISTO_SDK_LOG_FILE_SIZE = "DEMISTO_SDK_LOG_FILE_SIZE"
+DEMISTO_SDK_LOG_FILE_COUNT = "DEMISTO_SDK_LOG_FILE_COUNT"
+DEMISTO_SDK_LOG_NO_COLORS = "DEMISTO_SDK_LOG_NO_COLORS"
+# --- Environment Variables ---
+
+
 CAN_START_WITH_DOT_SLASH = "(?:./)?"
 NOT_TEST = "(?!Test)"
 
@@ -98,26 +137,6 @@
 MARKETPLACE_KEY_PACK_METADATA = "marketplaces"
 EVENT_COLLECTOR = "EventCollector"
 ASSETS_MODELING_RULE = "assetsmodelingrule"
-# ENV VARIABLES
-
-ENV_DEMISTO_SDK_MARKETPLACE = "DEMISTO_SDK_MARKETPLACE"
-DEMISTO_GIT_PRIMARY_BRANCH = os.getenv("DEMISTO_DEFAULT_BRANCH", "master")
-DEMISTO_GIT_UPSTREAM = os.getenv("DEMISTO_DEFAULT_REMOTE", "origin")
-DEMISTO_SDK_CI_SERVER_HOST = os.getenv("CI_SERVER_HOST", "gitlab.xdr.pan.local")
-DEMISTO_SDK_OFFICIAL_CONTENT_PROJECT_ID = os.getenv(
-    "CI_PROJECT_ID", "1061"
-)  # the default is the id of the content repo in gitlab.xdr.pan.local
-
-# authentication ENV VARIABLES
-DEMISTO_BASE_URL = "DEMISTO_BASE_URL"
-DEMISTO_USERNAME = "DEMISTO_USERNAME"
-DEMISTO_PASSWORD = "DEMISTO_PASSWORD"  # guardrails-disable-line
-DEMISTO_KEY = "DEMISTO_API_KEY"
-AUTH_ID = "XSIAM_AUTH_ID"
-XSIAM_TOKEN = "XSIAM_TOKEN"
-XSIAM_COLLECTOR_TOKEN = "XSIAM_COLLECTOR_TOKEN"
-DEMISTO_VERIFY_SSL = "DEMISTO_VERIFY_SSL"
-
 
 # Marketplaces
 
@@ -1932,7 +1951,6 @@ class ParameterType(Enum):
 
 FORMATTING_SCRIPT = "indicator-format"
 
-ENV_SDK_WORKING_OFFLINE = "DEMISTO_SDK_OFFLINE_ENV"
 DOCKERFILES_INFO_REPO = "demisto/dockerfiles-info"
 
 TEST_COVERAGE_DEFAULT_URL = f"https://storage.googleapis.com/{DEMISTO_SDK_MARKETPLACE_XSOAR_DIST_DEV}/code-coverage-reports/coverage-min.json"
@@ -1969,6 +1987,19 @@ class ParameterType(Enum):
 MARKDOWN_IMAGES_ARTIFACT_FILE_NAME = "markdown_images.json"
 SERVER_API_TO_STORAGE = "api/marketplace/file?name=content/packs"
 
+STRING_TO_BOOL_MAP = {
+    "y": True,
+    "1": True,
+    "yes": True,
+    "true": True,
+    "n": False,
+    "0": False,
+    "no": False,
+    "false": False,
+    "t": True,
+    "f": False,
+}
+
 
 #  date formats:
 ISO_TIMESTAMP_FORMAT = "%Y-%m-%dT%H:%M:%SZ"