Skip to content

Extract _resolve_profile to simplify config file loading#1333

Merged
simonfaltum merged 6 commits intomainfrom
simonfaltum/resolve-profile-refactor
Mar 20, 2026
Merged

Extract _resolve_profile to simplify config file loading#1333
simonfaltum merged 6 commits intomainfrom
simonfaltum/resolve-profile-refactor

Conversation

@simonfaltum
Copy link
Member

@simonfaltum simonfaltum commented Mar 16, 2026

Summary

Follow-up to #1325 (default_profile support). Applies the same refactoring we did in the Go SDK to improve readability and error messages.

Before: _known_file_config_loader used two booleans (has_explicit_profile, has_default_profile_setting) to track where the profile came from. The interaction between them encoded a three-way state (explicit, from settings, legacy fallback) that required careful reading to verify correctness. __settings__ handling was scattered across three locations in the method.

Now: A _resolve_profile static method handles all resolution logic and returns (profile, is_fallback). The loader no longer needs to know about __settings__ internals. The two booleans collapse into a single is_fallback flag.

Changes

  • Extract _resolve_profile(requested_profile, ini_file) as a static method that resolves the profile name through three sources: explicit, [__settings__].default_profile, or [DEFAULT] fallback.
  • Simplify _known_file_config_loader to use the resolved result. No more has_explicit_profile / has_default_profile_setting booleans.
  • Improve error messages: using __settings__ as a profile name (either explicitly or via default_profile) now returns "__settings__ is a reserved section name and cannot be used as a profile" instead of the generic "has no __settings__ profile configured".
  • Fix flaky SCIM pagination integration test: the raw GET /scim/v2/Groups was fetching the full result set just to read totalResults, causing read timeouts on large workspaces. Now passes count=1 since SCIM always returns totalResults regardless of page size.
  • Fix pre-existing formatting issues in 3 test files (test_auth.py, test_config.py, test_core.py) so the CI fmt check passes.

Why this matters

The three sources for a profile name (explicit, settings-based, legacy fallback) produce two distinct behaviors:

  1. Explicit and settings-based both require the profile to exist (error if missing) and record the active profile on cfg.profile.
  2. Legacy fallback silently returns if [DEFAULT] doesn't exist (unconfigured machine) and doesn't set cfg.profile.

A single is_fallback boolean captures this distinction. The previous two-boolean approach worked correctly but required tracing through their interactions to confirm that.

Test plan

  • All 8 existing test_default_profile.py tests pass (updated 2 tests to match the improved error messages).
  • No behavioral changes except the error message text for reserved section name usage.

This pull request was AI-assisted by Isaac.

The profile resolution logic in _known_file_config_loader used two
booleans (has_explicit_profile, has_default_profile_setting) to track
a three-way state: explicit profile, settings-based profile, or
legacy DEFAULT fallback. This made the control flow hard to follow.

Extract a _resolve_profile static method that returns (profile, is_fallback),
consolidating all __settings__ handling in one place. This also improves
error messages when __settings__ is used as a profile name (now says
"reserved section name" instead of "no profile configured").

Co-authored-by: Isaac
These 3 files were already unformatted on main. Including the fix
here so the CI fmt check passes.

Co-authored-by: Isaac
Co-authored-by: Isaac
The raw GET to /scim/v2/Groups was fetching the full result set just
to read totalResults. On workspaces with many groups, serializing the
full response exceeds the 60s read timeout, causing retries and
eventual failure after 5 minutes. SCIM always returns totalResults
regardless of page size, so count=1 is sufficient.

Co-authored-by: Isaac
@simonfaltum simonfaltum force-pushed the simonfaltum/resolve-profile-refactor branch from d48aee7 to 2faa56f Compare March 19, 2026 10:14
@github-actions
Copy link

If integration tests don't run automatically, an authorized user can run them manually by following the instructions below:

Trigger:
go/deco-tests-run/sdk-py

Inputs:

  • PR number: 1333
  • Commit SHA: 2faa56fd504889f30e100adbcfbf41cd54c5bdbd

Checks will be approved automatically on success.

@simonfaltum simonfaltum added this pull request to the merge queue Mar 20, 2026
Merged via the queue into main with commit 3a69982 Mar 20, 2026
17 checks passed
@simonfaltum simonfaltum deleted the simonfaltum/resolve-profile-refactor branch March 20, 2026 07:42
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