Skip to content
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 specifying empty list in provider and model allow/denylists #1185

Conversation

MaicoTimmerman
Copy link
Contributor

When setting c.AiExtension.allowed_providers = [], it acts like the default value None and will show all models.

We actually meant to not show any models at all. This is useful for when dynamically selecting models based on user/deployment.

@srdas
Copy link
Collaborator

srdas commented Jan 6, 2025

Related to Issue #913
Upstream issue in traitlets : ipython/traitlets#908
I tested the PR and it does not resolve the above issue.

@srdas srdas added the bug Something isn't working label Jan 6, 2025
@dlqqq
Copy link
Member

dlqqq commented Jan 6, 2025

To clarify, #913 documents a known issue where a user cannot specify a list containing a single item for a list configurable via traitlets. This was a regression introduced by traitlets==5.13.0, and @srdas verified that this regression continues to exist in the latest version of traitlets. We're exploring ways to remove our dependency on traitlets in v3.

However, specifying '--AiExtension.{allowed,blocked}_providers=[]' to jupyter-lab works, and does set the trait's value to an empty list in the backend. This can be verified from the server logs on init:

% jupyter lab '--AiExtension.allowed_providers=[]'
[I 2025-01-06 09:44:56.529 ServerApp] jupyter_ai | extension was successfully linked.
...
[I 2025-01-06 09:44:56.562 AiExtension] Configured provider allowlist: []

The code looks good to me. I'll approve and start the CI. @srdas You can help verify this PR if you have extra time.

@srdas
Copy link
Collaborator

srdas commented Jan 6, 2025

The code looks good to me. I'll approve and start the CI. @srdas You can help verify this PR if you have extra time.

Confirmed this works as intended using the approach above:
image

image

@dlqqq
Copy link
Member

dlqqq commented Jan 6, 2025

@srdas Thank you for verifying! Looks like the unit tests failed, so I've updated them to pass and assert this PR's new changes.

@dlqqq dlqqq changed the title Fix empty list in provider and model allow/deny-listing Fix specifying empty list in provider and model allow/denylists Jan 6, 2025
@dlqqq dlqqq merged commit 3f11712 into jupyterlab:main Jan 6, 2025
10 checks passed
@dlqqq
Copy link
Member

dlqqq commented Jan 6, 2025

@meeseeksdev please backport to 2.x

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants