Extract _resolve_profile to simplify config file loading#1333
Merged
simonfaltum merged 6 commits intomainfrom Mar 20, 2026
Merged
Extract _resolve_profile to simplify config file loading#1333simonfaltum merged 6 commits intomainfrom
simonfaltum merged 6 commits intomainfrom
Conversation
hectorcast-db
approved these changes
Mar 19, 2026
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
Co-authored-by: Isaac
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
d48aee7 to
2faa56f
Compare
|
If integration tests don't run automatically, an authorized user can run them manually by following the instructions below: Trigger: Inputs:
Checks will be approved automatically on success. |
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 free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
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.
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_loaderused 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_profilestatic 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 singleis_fallbackflag.Changes
_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._known_file_config_loaderto use the resolved result. No morehas_explicit_profile/has_default_profile_settingbooleans.__settings__as a profile name (either explicitly or viadefault_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".GET /scim/v2/Groupswas fetching the full result set just to readtotalResults, causing read timeouts on large workspaces. Now passescount=1since SCIM always returnstotalResultsregardless of page size.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:
cfg.profile.[DEFAULT]doesn't exist (unconfigured machine) and doesn't setcfg.profile.A single
is_fallbackboolean captures this distinction. The previous two-boolean approach worked correctly but required tracing through their interactions to confirm that.Test plan
test_default_profile.pytests pass (updated 2 tests to match the improved error messages).This pull request was AI-assisted by Isaac.