Skip to content

Removing option to specify a majority of arguments into AgentAuthConfiguration.__init__ positionally - #578

Open
Rodrigo Brandão (rodrigobr-msft) wants to merge 2 commits into
mainfrom
users/robrandao/config-init-args
Open

Removing option to specify a majority of arguments into AgentAuthConfiguration.__init__ positionally#578
Rodrigo Brandão (rodrigobr-msft) wants to merge 2 commits into
mainfrom
users/robrandao/config-init-args

Conversation

@rodrigobr-msft

Copy link
Copy Markdown
Contributor

This pull request introduces a minor change to the AgentAuthConfiguration class constructor to improve parameter handling. The change enforces the use of keyword-only arguments for parameters following the added *, which makes the API clearer and helps prevent accidental misuse.

  • Constructor improvement:
    • Added a * in the __init__ method signature of agent_auth_configuration.py to require that cert_pfx_file, connection_name, and federated_client_id are specified as keyword arguments.

Copilot AI lite review requested due to automatic review settings September 11, 2026 20:06
@rodrigobr-msft
Rodrigo Brandão (rodrigobr-msft) marked this pull request as ready for review September 11, 2026 20:06

Copilot AI 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.

🟡 Changes recommended

The public constructor change may break positional callers, and its new behavior lacks a focused regression test.

Get a fresh assessment by requesting another Copilot review.

Pull request overview

This PR makes AgentAuthConfiguration constructor parameters after client_secret keyword-only, improving clarity but potentially breaking positional callers.

Changes:

  • Adds * to the constructor signature.
  • Requires affected and subsequent options to be passed by keyword.
File summaries
File Summary
libraries/microsoft-agents-hosting-core/microsoft_agents/hosting/core/authorization/agent_auth_configuration.py Updates constructor parameter handling; requires compatibility planning and a focused keyword-only behavior test.
Review details

Suppressed comments (1)

libraries/microsoft-agents-hosting-core/microsoft_agents/hosting/core/authorization/agent_auth_configuration.py:126

  • The constructor tests cover successful keyword calls, but none verifies the new keyword-only contract. Add a focused test in test_auth_configuration.py that passes the affected options positionally and asserts TypeError, so future signature changes cannot silently restore or alter this API behavior.
        *,
  • Files reviewed: 1/1 changed files
  • Comments generated: 1
  • Review effort level: Lite

💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.

Copilot AI 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.

🟡 Changes recommended

Address positional compatibility or versioning and add regression coverage.

Get a fresh assessment by requesting another Copilot review.

Review details

Suppressed comments (1)

libraries/microsoft-agents-hosting-core/microsoft_agents/hosting/core/authorization/agent_auth_configuration.py:129

  • AgentAuthConfiguration is re-exported from both hosting.core and hosting.core.authorization, so this * is a breaking change for external callers that currently pass cert_pfx_file or any later option positionally: those calls now raise TypeError. Please either preserve the existing positional compatibility or handle this as an explicit breaking release with the corresponding migration/versioning; the PR currently presents it as a minor constructor improvement.
        *,
  • Files reviewed: 1/1 changed files
  • Comments generated: 1
  • Review effort level: Lite

Sign up for free to 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.

2 participants