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

Switch log.py's own logger to independant logger #861

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

zagpat
Copy link

@zagpat zagpat commented Nov 16, 2022

In case there is any kind of issue when setting up logger, use a specific logger for log calls. This avoids recursion issues when trying to operate "low level" calls on the logger we are actually working with. This specific logger logs to a temp file unrelated to the rest of sgtk log files to avoid file access issues and specific maintenance.

About the possible recusion issues (this works in debug mode):

  • you initialize the base file handler
  • there is an error while doing the rollover for the log file
  • there's a call to log.debug
  • log happens to be an instance of LogManager
  • it needs to initialize the base file handler

You may get stuck in a recursive loop.

In the same spirit of avoiding issues while working on "low level" operations, specifying exceptions we're catching to avoid catching exceptions unrelated to current process (and especially KeyboardInterrupt and the likes)

In case there is any kind of issue when setting up logger, use a
specific logger for log calls. This avoids recursion issues when trying
to operate "low level" calls on logger.
@codecov
Copy link

codecov bot commented Nov 16, 2022

Codecov Report

Merging #861 (e7b5bec) into master (5e0464f) will decrease coverage by 6.52%.
The diff coverage is 80.00%.

@@            Coverage Diff             @@
##           master     #861      +/-   ##
==========================================
- Coverage   79.40%   72.88%   -6.53%     
==========================================
  Files         177      177              
  Lines       19322    19333      +11     
==========================================
- Hits        15343    14090    -1253     
- Misses       3979     5243    +1264     
Impacted Files Coverage Δ
python/tank/log.py 88.82% <80.00%> (-1.06%) ⬇️
python/tank/commands/app_info.py 17.24% <0.00%> (-79.32%) ⬇️
python/tank/bootstrap/import_handler.py 18.18% <0.00%> (-73.74%) ⬇️
python/tank/commands/folders.py 20.00% <0.00%> (-69.10%) ⬇️
python/tank/commands/setup_project.py 17.32% <0.00%> (-66.29%) ⬇️
python/tank/commands/cache_yaml.py 31.57% <0.00%> (-52.64%) ⬇️
python/tank/commands/util.py 37.50% <0.00%> (-37.50%) ⬇️
python/tank/commands/core_localize.py 39.40% <0.00%> (-36.44%) ⬇️
python/tank/commands/unregister_folders.py 12.58% <0.00%> (-36.43%) ⬇️
python/tank/commands/tank_command.py 47.34% <0.00%> (-32.98%) ⬇️
... and 44 more

📣 We’re building smart automated test selection to slash your CI/CD build times. Learn more

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