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

fix for incorrect logging message format/args #330

Merged
merged 3 commits into from
Jul 26, 2023

Conversation

ankona
Copy link
Contributor

@ankona ankona commented Jul 26, 2023

A bug was introduced during pylint mitigation. The following error occurs when logging details of a freshly started colocated database:

--- Logging error ---
Traceback (most recent call last):
  File "/Users/ankona/.pyenv/versions/3.9.15/lib/python3.9/logging/__init__.py", line 1083, in emit
    msg = self.format(record)
  File "/Users/ankona/.pyenv/versions/3.9.15/lib/python3.9/logging/__init__.py", line 927, in format
    return fmt.format(record)
  File "/Users/ankona/.pyenv/versions/3.9.15/lib/python3.9/logging/__init__.py", line 663, in format
    record.message = record.getMessage()
  File "/Users/ankona/.pyenv/versions/3.9.15/lib/python3.9/logging/__init__.py", line 367, in getMessage
    msg = msg % self.args
TypeError: not all arguments converted during string formatting
Call stack:
  File "/Users/ankona/.pyenv/versions/3.9.15/lib/python3.9/runpy.py", line 197, in _run_module_as_main
    return _run_code(code, main_globals, None,
  File "/Users/ankona/.pyenv/versions/3.9.15/lib/python3.9/runpy.py", line 87, in _run_code
    exec(code, run_globals)
  File "/Users/ankona/code/ss/smartsim/_core/entrypoints/colocated.py", line 329, in <module>
    main(
  File "/Users/ankona/code/ss/smartsim/_core/entrypoints/colocated.py", line 209, in main
    logger.debug(
Message: '\n\nColocated database information\n\n\tIP Address(es): {}\n\tCommand: {}\n\n\n\t# of Database CPUs: {}'
Arguments: 
('127.0.0.1 127.0.0.1', 
 '/Users/ankona/code/ss/smartsim/_core/bin/redis-server /Users/ankona/code/ss/smartsim/_core/config/redis.conf --loadmodule /Users/ankona/code/ss/smartsim/_core/lib/redisai.so --port 6780 --logfile /Users/ankona/code/ss/tests/test_output/backends/test_dbscript/test_colocated_db_script/colocated_model-db.log --bind 127.0.0.1', 
 1)

This fix uses f-strings to ensure arguments are correctly converted during log message formatting.

Triggering

The error can be triggered by executing pytest tests/backends/test_dbscript.py

@ankona ankona marked this pull request as ready for review July 26, 2023 19:17
@ankona ankona requested review from mellis13 and ashao July 26, 2023 19:19
@codecov
Copy link

codecov bot commented Jul 26, 2023

Codecov Report

Merging #330 (0ce7aa9) into develop (211268e) will increase coverage by 0.11%.
Report is 1 commits behind head on develop.
The diff coverage is n/a.

Additional details and impacted files

Impacted file tree graph

@@             Coverage Diff             @@
##           develop     #330      +/-   ##
===========================================
+ Coverage    87.19%   87.30%   +0.11%     
===========================================
  Files           59       59              
  Lines         3522     3522              
===========================================
+ Hits          3071     3075       +4     
+ Misses         451      447       -4     

see 1 file with indirect coverage changes

Copy link
Contributor

@mellis13 mellis13 left a comment

Choose a reason for hiding this comment

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

Thanks for the fix!

@ankona ankona merged commit 4c741be into CrayLabs:develop Jul 26, 2023
10 checks passed
@MattToast MattToast added bug: minor A minor bug bug: major A major bug and removed bug: minor A minor bug labels Sep 13, 2023
@ankona ankona deleted the logfmt-fix branch April 4, 2024 19:32
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug: major A major bug
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants