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

Should allow to configure location of /generate error logs #487

Closed
krassowski opened this issue Nov 24, 2023 · 2 comments · Fixed by #490
Closed

Should allow to configure location of /generate error logs #487

krassowski opened this issue Nov 24, 2023 · 2 comments · Fixed by #490
Labels
enhancement New feature or request

Comments

@krassowski
Copy link
Member

krassowski commented Nov 24, 2023

Problem

#431 added generation of error log files for /generate. It writes to ./jupyter-ai-logs/ which one hand is nice because it is likely user accessible, but it is:

  • non-standard,
  • not configurable,
  • will attempt writing to a root location rather than the preferred dir.

For example, start:

jupyter lab --root-dir C:\ --preferred-dir C:\my\project\docs

Run /generate, see C:\jupyter-ai-logs was created, even though the main users "workspace" is in C:\my\project\docs

This means user will either:

  • be confused as they will not see jupyter-ai-logs and need to navigate a few levels up
  • will see an error if C:\ is not writable in itself (but only specific directories are allowed)

Proposed Solution

Add a traitlet making the path configurable.

Additional context

Probably a simple path trait would be sufficient, but FYI, jupyter-server and other apps have Application.logging_config which allows users to configure how logs are handled (see https://jupyter-server.readthedocs.io/en/latest/other/full-config.html).

@krassowski krassowski added the enhancement New feature or request label Nov 24, 2023
@dlqqq
Copy link
Member

dlqqq commented Nov 25, 2023

@krassowski Mike, these are great callouts as usual. I fully agree that there is room for improvement here.

The additional context you provided is valuable. We weren't aware that there was an existing trait that specifies logging configuration. We should definitely respect that if possible.

@krassowski
Copy link
Member Author

Thanks for a quick reply!

We weren't aware that there was an existing trait that specifies logging configuration. We should definitely respect that if possible.

I thought about it some more today. It is possible but not necessarily practical for this particular use case because by default logging_config does not configure a file handler and if you override the default you might end up conflicting with the settings downstream, and because it is using logging you may also need need to specify a custom formatter to make the output easy to read tracebacks. I still think it was worth checking that there is no existing error log path configurable we could fallback to (jupyterhub and jupyterlab-desktop have configurable error paths it but these are stack-specific and leave outside of the traitlets system).

I opened #490 with the proposed changes.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants