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

Number and size of log files can be set by configuration parameters #3795

Closed
gyandor opened this issue Aug 24, 2021 · 5 comments
Closed

Number and size of log files can be set by configuration parameters #3795

gyandor opened this issue Aug 24, 2021 · 5 comments
Assignees
Labels
cli enhancement New feature or request good_first_issue Straightforward + self-contained changes, good for new contributors! stale Issues that have gone stale

Comments

@gyandor
Copy link

gyandor commented Aug 24, 2021

Describe the feature

I would like to have configuration options to control the number and size of log files instead of having maximum 6 files 10MB each.

Describe alternatives you've considered

Periodic backup of current files to a different folder.

Who will this benefit?

We work with tons of models and log for 1 run exceeds 60 MB. If something goes wrong, we don't have the full log for investigation.

@gyandor gyandor added enhancement New feature or request triage labels Aug 24, 2021
@jtcohen6
Copy link
Contributor

@gyandor Sure, I'd welcome a contribution to make these configurable, while preserving the current values as defaults:

https://github.com/dbt-labs/dbt/blob/9e796671dd55d4781284d36c035d1db19641cd80/core/dbt/logger.py#L392-L393

These make sense to me as environment variables (DBT_LOGGER_MAX_SIZE, DBT_LOGGER_BACKUP_COUNT) and as profile configs. Personally, I don't see a ton of value in --logger-max-size or --logger-backup-count CLI flags, but since we're currently working toward consistent support for all three configuration options across all global configs (#2990), it would be best to follow the same pattern. The general precedence rule is CLI flag > env var > profile config.

Is this something you'd like to open a PR for?

@jtcohen6 jtcohen6 added cli good_first_issue Straightforward + self-contained changes, good for new contributors! and removed triage labels Aug 27, 2021
@sungchun12
Copy link
Contributor

@gyandor if you don't want to open a pull request for this, is it okay if I do?

@gyandor
Copy link
Author

gyandor commented Sep 8, 2021

@sungchun12 It's okay. Thank you in advance.

@sungchun12 sungchun12 self-assigned this Sep 8, 2021
@sungchun12
Copy link
Contributor

sungchun12 commented Sep 8, 2021

Approach Notes:

config:
  partial_parse: <true | false>
  use_colors: <true | false>
  printer_width: <integer>
  send_anonymous_usage_stats: <true | false>
  dbt_logger_max_size: <integer> # in megabyte increments only, the logic will handle the multiplication behind the scenes
  dbt_logger_backup_count: <integer>
  • Need to create priority conditions for: CLI flag > env var > profile config. I may be able to piggyback off existing logic.
  • Use BAR = os.environ.get('BAR') # None to prevent key errors

Open Questions

  • Should I put a max limit on max_size and backup_count?
  • Do I add to an existing test or build a new one?

@github-actions
Copy link
Contributor

github-actions bot commented Mar 8, 2022

This issue has been marked as Stale because it has been open for 180 days with no activity. If you would like the issue to remain open, please remove the stale label or comment on the issue, or it will be closed in 7 days.

@github-actions github-actions bot added the stale Issues that have gone stale label Mar 8, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
cli enhancement New feature or request good_first_issue Straightforward + self-contained changes, good for new contributors! stale Issues that have gone stale
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants