Skip to content

chore: dev to main merge - #135

Open
Saswato-Microsoft wants to merge 2 commits into
mainfrom
dev
Open

chore: dev to main merge#135
Saswato-Microsoft wants to merge 2 commits into
mainfrom
dev

Conversation

@Saswato-Microsoft

Copy link
Copy Markdown
Collaborator

Purpose

This pull request updates the logic for obtaining an Entra token in create_databricks_items_maag.py to use different Azure identity credentials based on the environment. The main change is to use DefaultAzureCredential with stricter requirements in development and ManagedIdentityCredential in other environments.

Credential selection improvements:

  • Updated _get_entra_token() to select credentials based on the APP_ENV environment variable: uses DefaultAzureCredential(require_envvar=True) in dev and ManagedIdentityCredential with AZURE_CLIENT_ID in other environments.

Does this introduce a breaking change?

  • Yes
  • No

How to Test

  • Get the code
git clone [repo-address]
cd [repo-name]
git checkout [branch-name]
  • Test the code

What to Check

Verify that the following are valid

  • ...

Other Information

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.

Pull request overview

This PR updates Databricks authentication token acquisition in create_databricks_items_maag.py to choose Azure Identity credentials based on the APP_ENV environment, aiming to use stricter developer-local auth and managed identity elsewhere.

Changes:

  • Updated _get_entra_token() to select an Azure Identity credential based on APP_ENV.
  • Introduced ManagedIdentityCredential usage (with AZURE_CLIENT_ID) for non-dev environments.
Suppressed comments (2)

infra/scripts/databricks/create_databricks_items_maag.py:319

  • DefaultAzureCredential(require_envvar=True) is very likely an invalid constructor argument for the azure-identity versions used in this repo (e.g., requirements.txt pins azure-identity==1.25.3). In that case it will raise TypeError and be silently swallowed by the broad except, causing token acquisition to fail in dev when Azure CLI isn't available.
            credential = DefaultAzureCredential(require_envvar=True)

infra/scripts/databricks/create_databricks_items_maag.py:325

  • In non-dev environments the PR description says to use ManagedIdentityCredential with AZURE_CLIENT_ID, but this code will fall back to system-assigned identity when AZURE_CLIENT_ID is unset and will also currently swallow any errors silently. Consider validating AZURE_CLIENT_ID and surfacing the failure reason so misconfiguration is actionable.
            credential = ManagedIdentityCredential(
                client_id=os.environ.get("AZURE_CLIENT_ID"))
        token = credential.get_token(f"{DATABRICKS_RESOURCE_ID}/.default")
        if token.token:
            return token.token

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

pass

# Method 2: azure-identity DefaultAzureCredential
# Method 2: environment-specific azure-identity credential
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.

4 participants