Skip to content

Support log configuration in the .env file - #230

Merged
Rodrigo Brandão (rodrigobr-msft) merged 17 commits into
mainfrom
users/robrandao/logging-with-config
Dec 8, 2025
Merged

Support log configuration in the .env file#230
Rodrigo Brandão (rodrigobr-msft) merged 17 commits into
mainfrom
users/robrandao/logging-with-config

Conversation

@rodrigobr-msft

Copy link
Copy Markdown
Contributor

This pull request introduces a small refactor to the configuration import path and adds a new placeholder for logging configuration. The main changes are grouped below:

Configuration import path update:

  • Changed the import of load_configuration_from_env in __init__.py to use the new config._load_configuration module path, improving code organization.

Logging setup:

  • Added a new configure_logging function in config/_configure_logging.py as a placeholder for future logging configuration.

CopilotAI review requested due to automatic review settings November 6, 2025 21:08

CopilotAI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull Request Overview

This PR refactors the configuration loading module path and introduces a placeholder for logging configuration functionality. The changes prepare the codebase for future logging configuration support while reorganizing the configuration-related code into a dedicated config package.

  • Moved configuration loading to a new config package structure
  • Added a placeholder configure_logging function for future logging setup

Reviewed Changes

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

FileDescription
config/_configure_logging.pyNew file containing placeholder function for logging configuration
__init__.pyUpdated import path to reference configuration loading from the new config package

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

CopilotAI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull Request Overview

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


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

@rodrigobr-msft
Rodrigo Brandão (rodrigobr-msft) marked this pull request as ready for review November 11, 2025 16:19
@cleemullins

Copy link
Copy Markdown
Contributor

Rodrigo Brandão (@rodrigobr-msft) The code here looks fine, but I'm not super-clear on why it's needed. Is there an issue that describes the problem, or is this just viewed as a fairly straightforward refactor?

@rodrigobr-msft

Copy link
Copy Markdown
ContributorAuthor

Rodrigo Brandão (@rodrigobr-msft) The code here looks fine, but I'm not super-clear on why it's needed. Is there an issue that describes the problem, or is this just viewed as a fairly straightforward refactor?

It's a desired feature as when debugging with customers, I have to give them 4 lines of code for them to copy and paste into their source code to enable logging. I was talking with a team that had a .NET config file, and they expected the same logging configuration found in .NET to work with Python. I think this is a good step towards parity in configuration and a small quality of life improvement for devs. I'm open to hearing other perspectives.

CopilotAI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull Request Overview

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


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

…config/_configure_logging.py
Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>

CopilotAI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull Request Overview

Copilot reviewed 4 out of 4 changed files in this pull request and generated 1 comment.

Comments suppressed due to low confidence (1)

libraries/microsoft-agents-activity/microsoft_agents/activity/config/_load_configuration.py:31

  • The function _configure_logging is called with side effects during configuration loading, but errors (e.g., invalid log level) could interrupt the configuration process. Consider handling logging configuration errors gracefully or documenting that invalid logging configuration will cause the entire configuration load to fail.

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

…config/_configure_logging.py
Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>

CopilotAI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull Request Overview

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

Comments suppressed due to low confidence (1)

libraries/microsoft-agents-activity/microsoft_agents/activity/config/_load_configuration.py:31

  • The logging configuration is called unconditionally within load_configuration_from_env, but if an invalid log level is provided in the configuration, a ValueError will be raised. This could cause the entire configuration loading to fail unexpectedly.

Consider wrapping the logging configuration in error handling to make the configuration loading more resilient:

try:
_configure_logging(result.get("LOGGING", {}))
exceptValueErrorase:
# Log the error or handle it appropriately# Could also re-raise if logging config failures should be fatalpass

Alternatively, document that logging configuration errors are intentionally fatal and will prevent application startup.


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

CopilotAI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

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


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

CopilotAI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Copilot reviewed 6 out of 6 changed files in this pull request and generated 3 comments.

Comments suppressed due to low confidence (1)

libraries/microsoft-agents-activity/microsoft_agents/activity/config/_load_configuration.py:31

  • The load_configuration_from_env function now has a side effect by calling _configure_logging, which modifies global logging state. This is unexpected behavior for a function that appears to only parse and return configuration data.

Consider:

  1. Documenting this side effect in the docstring
  2. Or returning the logging configuration in the result dictionary and letting the caller decide when to apply it
  3. Or renaming the function to reflect that it does more than just loading configuration

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

@rodrigobr-msft
Rodrigo Brandão (rodrigobr-msft) merged commit 5ce846f into mainDec 8, 2025
9 of 10 checks passed
@rodrigobr-msft
Rodrigo Brandão (rodrigobr-msft) deleted the users/robrandao/logging-with-config branch December 8, 2025 17:54
Sign up for freeto join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Allow logging to be configurable in the .env file

4 participants

@rodrigobr-msft@cleemullins@axelsrz