jupyter-ai icon indicating copy to clipboard operation
jupyter-ai copied to clipboard

`allowed_providers`/`blocked_providers` no longer work if only one is given

Open krassowski opened this issue 1 year ago • 6 comments
trafficstars

Description

allowed_providers/blocked_providers no longer work if only one is given. This is a pretty severe regression for administrators of tightly controlled deployments.

Reproduce

with traitlets 5.13.0 and 5.14.3:

jupyter lab --AiExtension.allowed_providers=X
[ AiExtension] Configured provider allowlist: None

with traitlets 5.12.0

jupyter lab --AiExtension.allowed_providers=X
[ AiExtension] Configured provider allowlist: ['X']

With all versions of traitlets:

jupyter lab --AiExtension.allowed_providers=X --AiExtension.allowed_providers=Y
[ AiExtension] Configured provider allowlist: ['X', 'Y']

Expected behavior

When adding this feature I added tests which would have caught it (https://github.com/jupyterlab/jupyter-ai/pull/415). Subsequently these were disabled (https://github.com/jupyterlab/jupyter-ai/pull/446).

I would appreciate if these tests could be restored.

Context

Extension 2.19.1 and earlier.

krassowski avatar Jul 26 '24 16:07 krassowski

Thank you for reporting this issue! I certainly did make a mistake by not following up and re-enabling those tests.

We plan to perform a release early next week (within 3-4 days), so I'll see if I can come up with a patch by then. Feel free to work on this issue in parallel as well if you'd like.

dlqqq avatar Jul 27 '24 15:07 dlqqq

The bug is in https://github.com/ipython/traitlets/issues/908. It would be trivial to revert the problematic change but might be hard to solve cleanly. I spent a lot of time debugging this and might not have time to work on a fix anytime soon.

In any case the only thing to do in juypter-ai is to increase test coverage for CLI options and re-enable skipped tests.

krassowski avatar Jul 27 '24 19:07 krassowski

This also affects other List traits with allow_none=True, namely --AiExtension.allowed_models and --AiExtension.blocked_models. I tested that Dict traits work fine on --AiExtension.model_parameters.

krassowski avatar Jul 27 '24 19:07 krassowski

traitlets==5.13.0 was released on PyPI on October 30, 2023. Since this has been an issue for a very long time, I don't think it would be wise to include a hasty patch for this issue in the upcoming release today. Adding a version ceiling like <5.13.0 could have unintended side effects. I'll focus on fixes/features for the next release, then work on this later this week. Feel free to ping me if you make any findings.

dlqqq avatar Jul 29 '24 18:07 dlqqq

I think that maybe we can fix it in jupyter-ai by specifying allow_none = False and adding a default=[]. I will try it another day. I agree no need to rush here as affected downstreams can pin trailets locally.

krassowski avatar Jul 29 '24 19:07 krassowski

traitlets==5.13.0 was released on PyPI on October 30, 2023

Yes, one week after https://github.com/jupyterlab/jupyter-ai/pull/415 was merged. And on November 8th the test was disabled in https://github.com/jupyterlab/jupyter-ai/pull/446, which I suspect was liekly because it was not passing locally because someone may have had the new version of traitlets installed locally but CI was still using the older/cached version.

krassowski avatar Jul 30 '24 08:07 krassowski