Skip to content

Add verbose log level for filenames being bundled #496

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

Merged
merged 6 commits into from
Oct 18, 2023

Conversation

mmarchetti
Copy link
Contributor

This PR adds a new log level, and emits messages at that level for each file that is added to a bundle. The new level is used when -v is present on the command line; use -vv for full debug logging.

Intent

Fixes #363

Type of Change

  • Bug Fix
  • New Feature
  • Breaking Change

Approach

Introduced a custom log level VERBOSE between INFO and DEBUG, and configure log level appropriately based on the verbosity. Change verbosity from a bool/flag to a counter/int.

Automated Tests

Our tests don't validate logged messages.

Directions for Reviewers

Deploy various content types (notebook, app/API, and manifest) with no verbosity, -v, and -vv. With -v, and -vv, you should see messages logged at level VERBOSE showing the files added to the bundle.

Checklist

  • I have updated CHANGELOG.md to cover notable changes.
  • I have updated all related GitHub issues to reflect their current state.

@mmarchetti mmarchetti requested a review from tdstein October 16, 2023 13:14
@github-actions
Copy link

github-actions bot commented Oct 16, 2023

☂️ Python Coverage

current status: ✅

Overall Coverage

Lines Covered Coverage Threshold Status
4302 2796 65% 0% 🟢

New Files

No new covered files...

Modified Files

File Coverage Status
rsconnect/actions.py 34% 🟢
rsconnect/bundle.py 80% 🟢
rsconnect/log.py 71% 🟢
rsconnect/main.py 55% 🟢
TOTAL 60% 🟢

updated for commit: 00903bc by action🐍

Copy link
Collaborator

@tdstein tdstein left a comment

Choose a reason for hiding this comment

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

LGTM.

I see copious amounts of logging output when setting -vv. Less on -v. And normal logging by default.

minor question; afaik VERBOSE isn't a standard log level. Should we be utilizing TRACE somehow?

@@ -135,7 +135,7 @@ def server_args(func):
type=click.Path(exists=True, file_okay=True, dir_okay=False),
help="The path to trusted TLS CA certificates.",
)
@click.option("--verbose", "-v", is_flag=True, help="Print detailed messages.")
@click.option("--verbose", "-v", count=True, help="Enable verbose output. Use -vv for very verbose (debug) output.")
Copy link
Collaborator

Choose a reason for hiding this comment

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

Nice.

Copy link
Collaborator

@tdstein tdstein left a comment

Choose a reason for hiding this comment

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

Meant to hit approve...

@mmarchetti
Copy link
Contributor Author

minor question; afaik VERBOSE isn't a standard log level. Should we be utilizing TRACE somehow?

I thought about that, but it didn't feel like trace output?

Copy link
Contributor

@kgartland-rstudio kgartland-rstudio left a comment

Choose a reason for hiding this comment

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

Validated. Deployed each content type + manifests with -v and -vv. All content deployed and shows appropriate log level ([VERBOSE] and [DEBUG]).

@mmarchetti mmarchetti merged commit 0c1c7e8 into master Oct 18, 2023
@mmarchetti mmarchetti deleted the mm-verbose-logging branch October 18, 2023 15:03
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.

Deploy Should Optionally List Packaged Files
3 participants