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

Sim Logging Bugs #373

Merged
merged 4 commits into from
Jan 8, 2025
Merged

Sim Logging Bugs #373

merged 4 commits into from
Jan 8, 2025

Conversation

lawhead
Copy link
Collaborator

@lawhead lawhead commented Jan 8, 2025

Overview

Fixed bugs in simulator logging.

Ticket

https://www.pivotaltracker.com/story/show/188758565

Contributions

  • Ensured that the configure_logger function clears any existing handlers before adding new ones.
  • Added unit tests for simulator artifact module to replicate the bug and confirm that it was resolved.
  • Fixed a bug in the sim task that occurred during cleanup.
  • Added documentation for simulation parameters. One of these controls whether the session.xlsx file is output.

Test

  • Ran all unit tests
  • Ran sample simulation

Discussion

Along with duplicate entries to stdout, the file handlers were also not cleared between runs. This resulted in log statements from subsequent runs being written to all the log files from previous runs. For a large number of runs this potentially resulted in huge log files for the first runs.

@lawhead lawhead requested a review from tab-cmd January 8, 2025 17:20
@lawhead
Copy link
Collaborator Author

lawhead commented Jan 8, 2025

I fixed all the bugs that I introduced in the unit tests. The remaining failures seem to be related to the integration tests, and particularly the language model, so I'll go ahead and merge now.

@lawhead lawhead merged commit 247a1b4 into 2.0.0 Jan 8, 2025
4 of 10 checks passed
@lawhead lawhead deleted the sim-logging-bug branch January 8, 2025 20:04
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