Skip to content

Add the auth.ProcessEnv function - #2404

Merged
shreyas-goenka merged 10 commits into
mainfrom
auth-process-env
Mar 4, 2025
Merged

Add the auth.ProcessEnv function#2404
shreyas-goenka merged 10 commits into
mainfrom
auth-process-env

Conversation

@shreyas-goenka

@shreyas-goenkashreyas-goenka commented Feb 28, 2025•
edited
Loading

Copy link
Copy Markdown
Contributor

Changes

This function provides all environment variables necessary to authenticate the downstream applications to the same credentials as the parent process.

It's used in #2278 and will also be useful for the databricks bundle exec command.

Tests

Unit test.

@shreyas-goenkashreyas-goenka changed the title auth process envAdd the auth.ProcessEnv functionFeb 28, 2025
@shreyas-goenka
shreyas-goenka changed the base branch from main to auth-env-varsFebruary 28, 2025 13:43
@shreyas-goenka
shreyas-goenka marked this pull request as ready for review February 28, 2025 13:47
pietern
pietern reviewed Mar 3, 2025
View reviewed changes
Comment threadinternal/testutil/env.go

os.Clearenv()
}

@pieternpieternMar 3, 2025

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.

Can you explain the nuance between the two different cleanup functions?

The distinction is not clear from the name (cleanup vs clear).

@shreyas-goenkashreyas-goenkaMar 3, 2025

Copy link
Copy Markdown
ContributorAuthor

Choose a reason for hiding this comment

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

Renamed this to NullEnvironment to make the nuance more clear.

@denikdenikMar 3, 2025

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.

I think NullEnvironment should follow naming of os.Clearenv() since that's essentially a wrapper: Clearenv(t* testing.T).

CleanupEnvironment can be renamed to SetMinimalEnvVars? However, I'd rather we did not do this in tests, this feels like it could break things.

andrewnester
andrewnester reviewed Mar 3, 2025
View reviewed changes
Comment threadlibs/auth/env.go Outdated
Comment threadlibs/auth/env.go Outdated
Comment threadlibs/auth/env.go
denik
denik reviewed Mar 3, 2025
View reviewed changes
Comment threadinternal/testutil/env.go
denik
denik reviewed Mar 3, 2025
View reviewed changes
Comment threadlibs/auth/env.go
Base automatically changed from auth-env-vars to mainMarch 3, 2025 15:06
@shreyas-goenka
shreyas-goenka added this pull request to the merge queueMar 4, 2025
Merged via the queue into main with commit 549b226Mar 4, 2025
@shreyas-goenka
shreyas-goenka deleted the auth-process-env branch March 4, 2025 14:54
denik pushed a commit that referenced this pull request May 20, 2026
@shreyas-goenka
Add theauth.ProcessEnvfunction (#2404)
cf15f14
## Changes
This function provides all environment variables necessary to
authenticate the downstream applications to the same credentials as the
parent process.
It's used in #2278 and will also
be useful for the `databricks bundle exec` command.
## Tests
Unit test.
Sign up for freeto join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@denikdenikdenik approved these changes
@pieternpieternAwaiting requested review from pietern
@andrewnesterandrewnesterAwaiting requested review from andrewnester

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

4 participants

@shreyas-goenka@pietern@denik@andrewnester