-
Notifications
You must be signed in to change notification settings - Fork 357
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
fix: cli fails when listing providers [DET-10127] #8903
Conversation
✅ Deploy Preview for determined-ui ready!
To edit notification comments on pull requests, go to your Netlify site configuration. |
harness/determined/cli/sso.py
Outdated
@@ -121,7 +121,7 @@ def list_providers(args: Namespace) -> None: | |||
except KeyError: | |||
raise EnterpriseOnlyError("No SSO providers data") | |||
|
|||
if len(sso_providers) == 0: | |||
if not sso_providers or len(sso_providers) == 0: |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
if not sso_providers:
should be sufficient. bool(<empty_list>)
evaluates to False
.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
fixed! thanks
Codecov ReportAttention: Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #8903 +/- ##
==========================================
- Coverage 47.53% 47.52% -0.01%
==========================================
Files 1066 1066
Lines 170248 170248
Branches 2235 2237 +2
==========================================
- Hits 80919 80915 -4
- Misses 89171 89175 +4
Partials 158 158
Flags with carried forward coverage won't be shown. Click here to find out more.
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM!
Description
In EE, the CLI returns an error when listing auth providers but auth is not being used. This is because we are always expecting a list, but we receive null in this case, hence the error
'NoneType' has no len()
. This fix updates the CLI to catch null values and not fail the length check.Test Plan
Run a devcluster on EE with the
oidc
config field populated with something like:Run the cli command
det auth list-providers
- the CLI should returnNo SSO providers found.
Commentary (optional)
Checklist
docs/release-notes/
.See Release Note for details.
Ticket