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

Remove config.json-related information #503

Merged

Conversation

andrii-i
Copy link
Collaborator

@andrii-i andrii-i commented Dec 5, 2023

Remove config.json-relared instructions added in PR #494 from Users readthedocs page. As discussed with @3coins, config.json should not be edited directly by people using Jupyter AI.

@andrii-i andrii-i added the documentation Improvements or additions to documentation label Dec 5, 2023
@krassowski
Copy link
Member

As discussed with @3coins, config.json should not be edited directly by people using Jupyter AI.

Indeed people using it locally should not need to touch configuration files 👍

However, people operating or administrating installations, and people using docker images (a large number of JupyterLab users start a clean image every day for example due to security concerns) may want to pre-specify the key.

My understanding is that such users should use environment variables, is it right? Would it be worth replacing this section with a guide for setting environment variables? From a quick glance it seems that elsewhere the documentation mentions (%env) which would only work for magics but not for chat (and still require manual user action). Instead I think that the variable needs to be set prior to the server start?

An alternative would be having a configurable traitlet but this is a separate discussion.

@3coins
Copy link
Collaborator

3coins commented Dec 5, 2023

@krassowski

However, people operating or administrating installations, and people using docker images (a large number of JupyterLab > users start a clean image every day for example due to security concerns) may want to pre-specify the key.

The Chat UI that this config refers to currently doesn't support the API keys as environment variables. I agree that we should unify the behavior for API keys input if present in env variables, but that's a separate effort.

Even for admins, I believe we should have a clear way to have them configure things rather than handling the config directly, for example we enable allowlists, blocklists etc. which are configured as traitlets and clearly something we expect as inputs. Similarly, PR #421 for default providers will provide admins a way to configure a language and embedding provider of their choice so that users don't have to configure them on their own; this should benefit people using docker images as well.

@andrii-i andrii-i merged commit 102a85a into jupyterlab:main Dec 5, 2023
7 checks passed
@andrii-i andrii-i deleted the remove-user-doc-config-instructions branch December 5, 2023 18:56
@andrii-i
Copy link
Collaborator Author

andrii-i commented Dec 5, 2023

Thank you for looking into this, @krassowski and @3coins.

I definitely agree that we need a non-UI way to specify API keys and other essential config fields for chat (and I thought that editing config directly would be a viable way to do it when creating #494). Created issue to track this:

JasonWeill pushed a commit to JasonWeill/jupyter-ai that referenced this pull request Dec 6, 2023
dlqqq pushed a commit to dlqqq/jupyter-ai that referenced this pull request Dec 8, 2023
dlqqq added a commit that referenced this pull request Dec 8, 2023
* Update Users section of the docs (#494)

* Update example of setting model provider and API key

* resize screenshots

* update chat settings screenshots

* Promote “Custom model providers” and “Customizing prompt templates” to subsections, move them to the bottom of the users doc page

* Update docs/source/users/index.md

Co-authored-by: Michał Krassowski <[email protected]>

* Update docs/source/users/index.md

Co-authored-by: Michał Krassowski <[email protected]>

* change double-backtick to single-backticks as they serve the same function

* move  Prompt templates sections to Developers page

* Update snapshots to use the same zoom level

* Update docs/source/developers/index.md

Co-authored-by: Jason Weill <[email protected]>

* move "Custom model providers" section to Dev page

* Update lang model choice screen to show OpenAI models mentioned in text around the screenshot

---------

Co-authored-by: Michał Krassowski <[email protected]>
Co-authored-by: Jason Weill <[email protected]>

* remove config.json-related information (#503)

* Base chat handler refactor for custom slash commands (#398)

* Adds attributes, starts adding to subclasses

* Consistent syntax

* Help for all handlers

* Fix slash ID error

* Iterate through entry points

* Fix typo in call to select()

* Moves config to magics, modifies extensions to attempt to load classes

* [pre-commit.ci] auto fixes from pre-commit.com hooks

for more information, see https://pre-commit.ci

* Moves config to proper location, improves error logging

* [pre-commit.ci] auto fixes from pre-commit.com hooks

for more information, see https://pre-commit.ci

* WIP: Updates per feedback, adds custom handler

* [pre-commit.ci] auto fixes from pre-commit.com hooks

for more information, see https://pre-commit.ci

* Removes redundant code, style fixes

* [pre-commit.ci] auto fixes from pre-commit.com hooks

for more information, see https://pre-commit.ci

* Removes unnecessary custom message

* Instantiates class

* Validates slash ID

* [pre-commit.ci] auto fixes from pre-commit.com hooks

for more information, see https://pre-commit.ci

* Consistent arguments to chat handlers

* [pre-commit.ci] auto fixes from pre-commit.com hooks

for more information, see https://pre-commit.ci

* Refactors to avoid intentionally unused params

* [pre-commit.ci] auto fixes from pre-commit.com hooks

for more information, see https://pre-commit.ci

* Updates docs, removes custom handler from source and config

* [pre-commit.ci] auto fixes from pre-commit.com hooks

for more information, see https://pre-commit.ci

* Renames process_message to match base class

* Adds needed parameter that had been deleted

* Joins lines in contributor doc

* Removes natural language routing type, which is not yet used

* Update docs/source/developers/index.md

Co-authored-by: Piyush Jain <[email protected]>

* Update docs/source/developers/index.md

Co-authored-by: Piyush Jain <[email protected]>

* Update docs/source/developers/index.md

Co-authored-by: Piyush Jain <[email protected]>

* Revises per @3coins, avoids Latinism

* Removes Configurable, since we do not yet have configurable traits

* Uses Literal for validation

* [pre-commit.ci] auto fixes from pre-commit.com hooks

for more information, see https://pre-commit.ci

---------

Co-authored-by: pre-commit-ci[bot] <66853113+pre-commit-ci[bot]@users.noreply.github.com>
Co-authored-by: Piyush Jain <[email protected]>

---------

Co-authored-by: Andrii Ieroshenko <[email protected]>
Co-authored-by: Michał Krassowski <[email protected]>
Co-authored-by: Jason Weill <[email protected]>
Co-authored-by: pre-commit-ci[bot] <66853113+pre-commit-ci[bot]@users.noreply.github.com>
Co-authored-by: Piyush Jain <[email protected]>
dbelgrod pushed a commit to dbelgrod/jupyter-ai that referenced this pull request Jun 10, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
documentation Improvements or additions to documentation
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants