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

Bring back the info output on the dagster-webserver CLI #16357

Merged
merged 1 commit into from
Sep 7, 2023

Conversation

gibsondan
Copy link
Member

Summary:
since uvicorn INFO output is spammy but dagster INFO output is not (and adding a WARNING about what port you're running on seems like overkill), split it out into two separate CLI args: --dagster-log-level and --uvicorn-log-level. Not in love with this solution but it was the best I could find that still allowed customziing each of the various knobs while leaving the default dagster dev experience reasonable.

Test Plan:
Run default 'dagster dev' command - see info level dagster logs in dagster-webserver, but no uvicorn logs
Run 'dagster-webserver --uvicorn-log-level INFO --dagster-log-level WARNING - see just uvicorn logs at the INFO level Run 'dagster dev --log-level DEBUG - see it affect both daemon output and dagster webserver logs, but not uvicorn traces

Summary & Motivation

How I Tested These Changes

Summary:
since uvicorn INFO output is spammy but dagster INFO output is not (and adding a WARNING about what port you're running on seems like overkill), split it out into two separate CLI args: --dagster-log-level and --uvicorn-log-level. Not in love with this solution but it was the best I could find that still allowed customziing each of the various knobs while leaving the default dagster dev experience reasonable.

Test Plan:
Run default 'dagster dev' command - see info level dagster logs in dagster-webserver, but no uvicorn logs
Run 'dagster-webserver --uvicorn-log-level INFO --dagster-log-level WARNING` - see just uvicorn logs at the INFO level
Run 'dagster dev --log-level DEBUG` - see it affect both daemon output and dagster webserver logs, but not uvicorn traces
@gibsondan gibsondan merged commit cf328ff into master Sep 7, 2023
1 check was pending
@gibsondan gibsondan deleted the moarloggers branch September 7, 2023 15:20
gibsondan added a commit that referenced this pull request Sep 7, 2023
Summary:
since uvicorn INFO output is spammy but dagster INFO output is not (and
adding a WARNING about what port you're running on seems like overkill),
split it out into two separate CLI args: --dagster-log-level and
--uvicorn-log-level. Not in love with this solution but it was the best
I could find that still allowed customziing each of the various knobs
while leaving the default dagster dev experience reasonable.

Test Plan:
Run default 'dagster dev' command - see info level dagster logs in
dagster-webserver, but no uvicorn logs
Run 'dagster-webserver --uvicorn-log-level INFO --dagster-log-level
WARNING` - see just uvicorn logs at the INFO level
Run 'dagster dev --log-level DEBUG` - see it affect both daemon output
and dagster webserver logs, but not uvicorn traces

## Summary & Motivation

## How I Tested These Changes
zyd14 pushed a commit to zyd14/dagster that referenced this pull request Sep 15, 2023
…6357)

Summary:
since uvicorn INFO output is spammy but dagster INFO output is not (and
adding a WARNING about what port you're running on seems like overkill),
split it out into two separate CLI args: --dagster-log-level and
--uvicorn-log-level. Not in love with this solution but it was the best
I could find that still allowed customziing each of the various knobs
while leaving the default dagster dev experience reasonable.

Test Plan:
Run default 'dagster dev' command - see info level dagster logs in
dagster-webserver, but no uvicorn logs
Run 'dagster-webserver --uvicorn-log-level INFO --dagster-log-level
WARNING` - see just uvicorn logs at the INFO level
Run 'dagster dev --log-level DEBUG` - see it affect both daemon output
and dagster webserver logs, but not uvicorn traces

## Summary & Motivation

## How I Tested These Changes
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants