Dse/log standardization prod 14939 #411

sellisd · 2025-11-14T15:46:42Z

No description provided.

Removed duplicate code. Configure the location of logs with an option instead of environment variable

Copilot

Pull Request Overview

This pull request focuses on standardizing logging across the Babylon codebase to improve consistency and maintainability. The changes include migrating from hard-coded logger names to module-based naming, removing redundant log prefixes, and enhancing the logging infrastructure.

Key Changes:

Standardized logger initialization from getLogger("Babylon") to getLogger(__name__) across all modules for better log traceability
Removed redundant prefixes ([api], [powerbi], [babylon]) from log messages since the logger name now includes module information
Refactored logging setup in main.py with dedicated file handlers for standard logs and errors, plus a new --log-path CLI option

Reviewed Changes

Copilot reviewed 93 out of 93 changed files in this pull request and generated 12 comments.

Show a summary per file

File	Description
Babylon/main.py	Introduced `setup_logging()` function with file handlers and new `--log-path` option; removed test mode parameter
Babylon/utils/response.py	Changed logger name to `__name__` and added info logging for YAML file dumps
Babylon/test/api/datasets/test_datasets_service.py	Updated test expectations to match API response format changes (list instead of dict) and added workspace-id parameter
Babylon/commands/macro/*.py	Standardized logger initialization and removed log prefixes from messages
Babylon/commands/api/*/.py	Standardized logger names, removed JSON dumps from logs, removed redundant prefixes, and corrected spelling errors

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Babylon/main.py

Copilot · 2025-11-14T15:52:05Z

Babylon/commands/api/solutions/security/get.py

 from Babylon.commands.api.solutions.services.solutions_security_svc import SolutionSecurityService

-logger = getLogger("Babylon")
+logger = getLogger("__name__")


The logger is incorrectly initialized with "__name__" as a string literal instead of the __name__ variable. This will result in all log messages from this module showing "name" instead of the actual module name.

Change to: logger = getLogger(__name__)

Copilot · 2025-11-14T15:52:05Z

Babylon/commands/api/solutions/security/set_default.py

    default_security = response.json()
    logger.info(json.dumps(default_security, indent=2))
-    logger.info(f"[api] default RBAC access successfully setted with role {[role]}")
+    logger.info(f"default RBAC access successfully setted with role {[role]}")


The word "setted" should be corrected to "set" (proper past participle of "set").

Copilot · 2025-11-14T15:52:05Z

Babylon/commands/api/solutions/security/set_default.py

 from Babylon.commands.api.solutions.services.solutions_security_svc import SolutionSecurityService

-logger = getLogger("Babylon")
+logger = getLogger("__name__")


The logger is incorrectly initialized with "__name__" as a string literal instead of the __name__ variable. This will result in all log messages from this module showing "name" instead of the actual module name.

Change to: logger = getLogger(__name__)

Copilot · 2025-11-14T15:52:05Z

Babylon/commands/api/solutions/runTemplates/get_all.py

        return CommandResponse.fail()
    run_templates = response.json()
-    logger.info(json.dumps(run_templates, indent=2))
+    logger.info(f"Retrieving all runtemplates in the solution {[service_state['api']['solution_id']]}")


The log message is placed after the API call returns successfully, which changes the logging behavior. The message says "Retrieving" (present tense) but appears after retrieval is complete. Consider either:

Moving this line before line 39 (before the API call) to maintain the original intent, or

Changing the message to past tense: "Retrieved all runtemplates from solution..."

Copilot · 2025-11-14T15:52:06Z

Babylon/commands/api/organizations/get_all.py

        env.store_state_in_cloud(state)
-    logger.info(json.dumps(organizations, indent=2))
+    ids = [o["id"] for o in organizations]
+    logger.info(f"Retrieved {ids} organizations details")


The log message format is incorrect. The f-string is trying to insert a list of IDs directly before the word "organizations", resulting in output like "Retrieved ['id1', 'id2'] organizations details".

Consider changing to: logger.info(f"Retrieved {len(ids)} organizations details: {ids}")

Suggested change

logger.info(f"Retrieved {ids} organizations details")

logger.info(f"Retrieved {len(ids)} organizations details: {ids}")

Copilot · 2025-11-14T15:52:07Z

Babylon/commands/api/solutions/security/update.py

 from Babylon.commands.api.solutions.services.solutions_security_svc import SolutionSecurityService

-logger = getLogger("Babylon")
+logger = getLogger("__name__")


The logger is incorrectly initialized with "__name__" as a string literal instead of the __name__ variable. This will result in all log messages from this module showing "name" instead of the actual module name.

Change to: logger = getLogger(__name__)

Copilot · 2025-11-14T15:52:07Z

Babylon/commands/api/solutions/security/get_users.py

 from Babylon.commands.api.solutions.services.solutions_security_svc import SolutionSecurityService

-logger = getLogger("Babylon")
+logger = getLogger("__name__")


The logger is incorrectly initialized with "__name__" as a string literal instead of the __name__ variable. This will result in all log messages from this module showing "name" instead of the actual module name.

Change to: logger = getLogger(__name__)

Copilot · 2025-11-14T15:52:07Z

Babylon/commands/api/solutions/security/get_all.py

 from Babylon.commands.api.solutions.services.solutions_security_svc import SolutionSecurityService

-logger = getLogger("Babylon")
+logger = getLogger("__name__")


The logger is incorrectly initialized with "__name__" as a string literal instead of the __name__ variable. This will result in all log messages from this module showing "name" instead of the actual module name.

Change to: logger = getLogger(__name__)

Copilot · 2025-11-14T15:52:07Z

Babylon/commands/api/datasets/get_all.py

    if len(datasets) and filter:
        datasets = jmespath.search(filter, datasets)
-    logger.info(json.dumps(datasets, indent=2))
+    logger.info(f"Retrieved {[d.get('id') for d in datasets]} datastes")


Typo in log message: "datastes" should be "datasets".

Suggested change

logger.info(f"Retrieved {[d.get('id') for d in datasets]} datastes")

logger.info(f"Retrieved {[d.get('id') for d in datasets]} datasets")

Copilot

Pull Request Overview

Copilot reviewed 94 out of 94 changed files in this pull request and generated 4 comments.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Babylon/commands/api/organizations/get.py

Copilot · 2025-11-14T16:10:11Z

Babylon/main.py

+    log_file_handler.setLevel(logging.INFO)
+    error_file_handler = logging.FileHandler(log_path / "babylon.error")
+    error_file_handler.setLevel(logging.WARNING)
+    logging.basicConfig(format="%(asctime)s:%(levelname)s:%(name)s:%(message)s",


Inconsistent logging format: The format string uses %(name)s to include the logger name, but line 19 changes the root logger from getLogger("Babylon") to getLogger() (root logger). This means all loggers will now use their module names (e.g., "Babylon.commands.api.workspaces.create") which could make the log format string %(name)s produce very long entries. Consider adjusting the format to truncate or use a shorter format for the name.

Copilot · 2025-11-14T16:10:11Z

Babylon/main.py



+def setup_logging(log_path: pathlibPath = pathlibPath.cwd()) -> None:
+    import click  # noqa F401


Unused import: import click # noqa F401 is imported but not used in this function. The import appears to be only for the tracebacks_suppress parameter, but it's already imported at the top level of the module.

Copilot · 2025-11-14T16:10:11Z

Babylon/commands/api/runs/logs.py

+    print(response.text)
+    return CommandResponse.success(response.text)


Changed return type from JSON to text: The function now returns response.text instead of response.json(). This changes the API behavior - callers expecting JSON data will now receive a string. This also adds a print() statement that outputs directly to stdout, which may not be desired in all contexts (e.g., when output is redirected or when running in non-interactive mode).

refactor: simplify logging setup

ec70119

Removed duplicate code. Configure the location of logs with an option instead of environment variable

sellisd requested a review from Copilot November 14, 2025 15:46

Copilot started reviewing on behalf of sellisd November 14, 2025 15:47 View session

Copilot finished reviewing on behalf of sellisd November 14, 2025 15:50

Copilot AI reviewed Nov 14, 2025

View reviewed changes

sellisd requested a review from Copilot November 14, 2025 16:07

Copilot started reviewing on behalf of sellisd November 14, 2025 16:07 View session

Copilot finished reviewing on behalf of sellisd November 14, 2025 16:09

Copilot AI reviewed Nov 14, 2025

View reviewed changes

refactor: organization logs

205d019

sellisd force-pushed the DSE/log_standardization_PROD-14939 branch from 7c62e9e to 205d019 Compare November 14, 2025 16:18

sellisd marked this pull request as ready for review November 14, 2025 16:19

sellisd requested a review from MohcineTor November 14, 2025 16:19

	logger.info(f"Retrieved {ids} organizations details")
	logger.info(f"Retrieved {len(ids)} organizations details: {ids}")

	logger.info(f"Retrieved {[d.get('id') for d in datasets]} datastes")
	logger.info(f"Retrieved {[d.get('id') for d in datasets]} datasets")



		def setup_logging(log_path: pathlibPath = pathlibPath.cwd()) -> None:
		import click # noqa F401

		print(response.text)
		return CommandResponse.success(response.text)

Dse/log standardization prod 14939 #411

Are you sure you want to change the base?

Dse/log standardization prod 14939 #411

Uh oh!

Conversation

sellisd commented Nov 14, 2025

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull Request Overview

Key Changes:

Reviewed Changes

Uh oh!

Uh oh!

Copilot AI Nov 14, 2025

Choose a reason for hiding this comment

Uh oh!

Copilot AI Nov 14, 2025

Choose a reason for hiding this comment

Uh oh!

Copilot AI Nov 14, 2025

Choose a reason for hiding this comment

Uh oh!

Copilot AI Nov 14, 2025

Choose a reason for hiding this comment

Uh oh!

Copilot AI Nov 14, 2025

Choose a reason for hiding this comment

Uh oh!

Copilot AI Nov 14, 2025

Choose a reason for hiding this comment

Uh oh!

Copilot AI Nov 14, 2025

Choose a reason for hiding this comment

Uh oh!

Copilot AI Nov 14, 2025

Choose a reason for hiding this comment

Uh oh!

Copilot AI Nov 14, 2025

Choose a reason for hiding this comment

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull Request Overview

Uh oh!

Uh oh!

Copilot AI Nov 14, 2025

Choose a reason for hiding this comment

Uh oh!

Copilot AI Nov 14, 2025

Choose a reason for hiding this comment

Uh oh!

Copilot AI Nov 14, 2025

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants