Movatterモバイル変換

[0]ホーム
URL:

Skip to content

Navigation Menu

Sign in
Appearance settings

Search code, repositories, users, issues, pull requests...

Provide feedback

We read every piece of feedback, and take your input very seriously.

Saved searches

Use saved searches to filter your results more quickly

 Sign up
Appearance settings

Create Shell Log Storage to Print Warnings in CLI#19974

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 ourterms of service andprivacy statement. We’ll occasionally send you account related emails.

Already on GitHub?Sign in to your account

Open
maiadegraaf wants to merge3 commits intoduckdb:main
base:main
Choose a base branch
Loading
frommaiadegraaf:print_warnings_in_cli

Conversation

@maiadegraaf
Copy link
Contributor

@maiadegraafmaiadegraaf commentedNov 28, 2025•
edited
Loading

Screenshot 2025-11-28 at 11 03 19

Add a basicShellLogStorage to print logs to the CLI.

Currently auto-enabled.

Releted issue:https://github.com/duckdblabs/duckdb-internal/issues/5153

taniabogatsch reacted with hooray emojicarlopi reacted with rocket emoji
Copy link
Contributor

@taniabogatschtaniabogatsch left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others.Learn more.

Super cool - just a small request. :)

Copy link
Contributor

@samansminksamansmink left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others.Learn more.

Looks great@maiadegraaf! have a few comments

@samansmink
Copy link
Contributor

After long bikeshedding sesh with@carlopi@maiadegraaf and me:

  • We'll log a warning always onenable_logging that explains users that their warning settings are changing, this helps solve carlo's concerns
maiadegraaf reacted with thumbs up emoji

@carlopi
Copy link
Contributor

I think this is great to have.

Rephrasing of my concerns with the initial version of the PR:

  1. Disabling Warning system without explicit consent from users looks off
  2. CallingCALL enable_logging(<...>) will essentially shut down the Warning system
  3. 1+2 not nice together, how to? Print a Warning on disabling the Warnings!

We might want to look more properly at enable logging / profiling UX, but that's for another PR.

samansmink and maiadegraaf reacted with thumbs up emoji

@duckdb-draftbotduckdb-draftbot marked this pull request as draftNovember 28, 2025 16:11
@maiadegraafmaiadegraaf marked this pull request as ready for reviewNovember 28, 2025 16:12
Sign up for freeto join this conversation on GitHub. Already have an account?Sign in to comment

Reviewers

@MytherinMytherinAwaiting requested review from Mytherin

2 more reviewers

@samansminksamansminksamansmink left review comments

@taniabogatschtaniabogatschtaniabogatsch requested changes

Reviewers whose approvals may not affect merge requirements

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

4 participants

@maiadegraaf@samansmink@carlopi@taniabogatsch
[8]ページ先頭
©2009-2025 Movatter.jp