Uh oh!
There was an error while loading. Please reload this page.
Strip trailing slash from host in auth and configure commands - #4633
Merged
simonfaltum merged 3 commits intoMar 12, 2026
Merged
Conversation
varundeepsaini
requested review from
andrewnester, anton-107, denik, pietern and shreyas-goenka
as code ownersMarch 3, 2026 09:58
simonfaltum
left a comment
Member
There was a problem hiding this comment.
Had an extra look. Would you be up for adding some tests for the auth token path?
The setHostAndAccountId function is also called from cmd/auth/token.go:166 and token.go:426
| // Strip trailing slash from host. Users often copy-paste URLs with a | ||
| // trailing slash from browsers or docs; normalize here to avoid errors | ||
| // from downstream validation in the SDK. | ||
| authArguments.Host = strings.TrimRight(authArguments.Host, "/") |
Member
There was a problem hiding this comment.
Using strings.TrimRight(input, "/"), which strips all trailing / characters from the right side of the string, character by character. I think this is subtly wrong for edge cases.
Look at the test expectation on line 32 of host_test.go: {"https://", "https:"},
TrimRight("https://", "/") strips both trailing slashes from https://, producing "https:". That's clearly broken. The input https:// is an invalid host either way, but the function is now actively mangling it in a confusing way. I think the correct function to use is strings.TrimSuffix(input, "/") which only removes a single trailing /.
For https://example.com/ both functions produce the same result, but TrimRight is a future trap for multi-slash paths or edge cases, I think?
Uh oh!
There was an error while loading. Please reload this page.
Uh oh!
There was an error while loading. Please reload this page.
varundeepsainiforce-pushed
the
fix-auto-strip-backslash-auth-host
branch
from
March 3, 2026 12:47
16c3003 to
00ed36cComparevarundeepsainiforce-pushed
the
fix-auto-strip-backslash-auth-host
branch
2 times, most recently
from
March 3, 2026 12:51
dfd5ed1 to
81f2d25Compare
Signed-off-by: Varun Deep Saini <varun.23bcs10048@ms.sst.scaler.com>
varundeepsainiforce-pushed
the
fix-auto-strip-backslash-auth-host
branch
from
March 3, 2026 12:58
81f2d25 to
5db8392CompareContributorAuthor
@simonfaltum could you run the ci ? |
Contributor
An authorized user can trigger integration tests manually by following the instructions below: Trigger: Inputs:
Checks will be approved automatically on success. |
andrewnester
approved these changes
Mar 10, 2026
Collaborator
Commit: 5149e46
16 interesting tests: 7 KNOWN, 7 SKIP, 2 flaky
Top 21 slowest tests (at least 2 minutes):
|
varundeepsaini
commented
Mar 10, 2026
ContributorAuthor
@andrewnester could we merge thiis ? |
Uh oh!
There was an error while loading. Please reload this page.
rauchy pushed a commit
that referenced
this pull request
Mar 17, 2026
Fixes#4628 ## Summary - Strip trailing slash from host URL in `setHostAndAccountId` (used by `auth login` and `auth token`) - Strip trailing slash in `normalizeHost` (used by `databricks configure`) ## Test plan - [x] Added test cases for trailing slash stripping in `setHostAndAccountId` (flag and positional arg) - [x] Updated `normalizeHost` test expectations for trailing slash inputs - [x] Verified all existing tests pass Signed-off-by: Varun Deep Saini <varun.23bcs10048@ms.sst.scaler.com> Co-authored-by: simon <simon.faltum@databricks.com>
deco-sdk-taggingBot
added a commit
that referenced
this pull request
Mar 18, 2026
## Release v0.295.0 ### Notable Changes * Databricks Asset Bundles have been renamed to Declarative Automation Bundles (DABs). This is a non-breaking change; no code or configuration modifications are required. See the [FAQ](https://docs.databricks.com/aws/en/dev-tools/bundles/faqs#why-was-databricks-asset-bundles-renamed-to-declarative-automation-bundles). * Add `bundle.engine` config setting to select the deployment engine (`terraform` or [`direct`](https://docs.databricks.com/aws/en/dev-tools/bundles/direct)). The `bundle.engine` setting takes precedence over the `DATABRICKS_BUNDLE_ENGINE` environment variable. When the configured engine doesn't match existing deployment state, a warning is issued and the existing engine is used ([#4749](#4749), [#4782](#4782)) ### CLI * Add `databricks auth switch` command for setting the default profile ([#4651](#4651)) * Add positional argument support to `auth logout` ([#4744](#4744)) * Strip trailing slash from host in `auth login`, `auth token`, and `configure` commands ([#4633](#4633)) ### Bundles * Standardize `personal_schemas` enum across bundle templates ([#4401](#4401)) * engine/direct: Fix permanent drift on experiment name field ([#4627](#4627)) * engine/direct: Fix permissions state path to match input config schema ([#4703](#4703)) * Add default project name and success message to default-scala template ([#4661](#4661)) * Skip enum validation for unresolved variable references ([#4752](#4752)) * engine/direct: Support references to/from grants ([#4774](#4774))
denik pushed a commit
that referenced
this pull request
May 20, 2026
Fixes#4628 ## Summary - Strip trailing slash from host URL in `setHostAndAccountId` (used by `auth login` and `auth token`) - Strip trailing slash in `normalizeHost` (used by `databricks configure`) ## Test plan - [x] Added test cases for trailing slash stripping in `setHostAndAccountId` (flag and positional arg) - [x] Updated `normalizeHost` test expectations for trailing slash inputs - [x] Verified all existing tests pass Signed-off-by: Varun Deep Saini <varun.23bcs10048@ms.sst.scaler.com> Co-authored-by: simon <simon.faltum@databricks.com>
denik pushed a commit
that referenced
this pull request
May 20, 2026
## Release v0.295.0 ### Notable Changes * Databricks Asset Bundles have been renamed to Declarative Automation Bundles (DABs). This is a non-breaking change; no code or configuration modifications are required. See the [FAQ](https://docs.databricks.com/aws/en/dev-tools/bundles/faqs#why-was-databricks-asset-bundles-renamed-to-declarative-automation-bundles). * Add `bundle.engine` config setting to select the deployment engine (`terraform` or [`direct`](https://docs.databricks.com/aws/en/dev-tools/bundles/direct)). The `bundle.engine` setting takes precedence over the `DATABRICKS_BUNDLE_ENGINE` environment variable. When the configured engine doesn't match existing deployment state, a warning is issued and the existing engine is used ([#4749](#4749), [#4782](#4782)) ### CLI * Add `databricks auth switch` command for setting the default profile ([#4651](#4651)) * Add positional argument support to `auth logout` ([#4744](#4744)) * Strip trailing slash from host in `auth login`, `auth token`, and `configure` commands ([#4633](#4633)) ### Bundles * Standardize `personal_schemas` enum across bundle templates ([#4401](#4401)) * engine/direct: Fix permanent drift on experiment name field ([#4627](#4627)) * engine/direct: Fix permissions state path to match input config schema ([#4703](#4703)) * Add default project name and success message to default-scala template ([#4661](#4661)) * Skip enum validation for unresolved variable references ([#4752](#4752)) * engine/direct: Support references to/from grants ([#4774](#4774))
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for freeto join this conversation on GitHub.
Already have an account?
Sign in to comment
4 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Fixes#4628
Summary
setHostAndAccountId(used byauth loginandauth token)normalizeHost(used bydatabricks configure)Test plan
setHostAndAccountId(flag and positional arg)normalizeHosttest expectations for trailing slash inputs