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

Add --event-buffer-size flag, recreate EVENT_HISTORY once configured #4416

Merged
merged 1 commit into from
Dec 3, 2021

Conversation

jtcohen6
Copy link
Contributor

@jtcohen6 jtcohen6 commented Dec 3, 2021

Follow-on to #4411

  • Add --event-buffer-size flag to main.py
  • When GLOBAL_HISTORY is being created right now, flags have not yet been resolved, so it's just using the default value set in the flags module (10k). We need to recreate global EVENT_HISTORY once all configs are available.

The downside: We might lose 1-2 events that are fired before we call setup_event_logger(). Currently, this is just deprecation warnings for renamed configs in dbt_project.yml.

The problem: Currently, setup_event_logger is being called much later than it should be from main.py, because it needs to wait for log_path to be resolved (based on config in dbt_project.yml).

Flags are resolved earlier than that, so we should refactor this to either:

  • Split setup_event_logger into different functions for setting up global EVENT_HISTORY + STDOUT_LOG vs. global FILE_LOG, since only the latter needs to know log_path
  • Set up FILE_LOG to use RotatingFileHandler with delay + emit, i.e. buffer events until it knows where log_path is, then drop them in the file. This is the approach we used to take with the legacy logger.

Checklist

  • I have signed the CLA
  • I have run this code in development and it appears to resolve the stated issue
  • This PR includes tests, or tests are not required/relevant for this PR
  • I have updated the CHANGELOG.md and added information about my change

@jtcohen6 jtcohen6 requested review from leahwicz and iknox-fa December 3, 2021 11:32
@cla-bot cla-bot bot added the cla:yes label Dec 3, 2021
@jtcohen6 jtcohen6 merged commit b67d5f3 into main Dec 3, 2021
@jtcohen6 jtcohen6 deleted the add-event-buffer-flag branch December 3, 2021 13:36
leahwicz added a commit that referenced this pull request Dec 3, 2021
@nathaniel-may nathaniel-may mentioned this pull request Dec 6, 2021
26 tasks
iknox-fa pushed a commit that referenced this pull request Feb 8, 2022
automatic commit by git-black, original commits:
  b67d5f3
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants