FazBrowse GitHub Viewer | Trending |
URL:
| Home
Tools: [Download Repo ZIP]   [Original HTTPS Page]

gh-103606: Improve error message from logging.config.FileConfig by Agent-Hellboy · Pull Request #103628 · python/cpython · GitHub

/ cpython Public

gh-103606: Improve error message from logging.config.FileConfig - #103628

Merged
vsajip merged 14 commits into
python:mainfrom
Agent-Hellboy:fix-issue-103606
May 18, 2023
ConversationCommits (14)ChecksFiles changed
Merged

gh-103606: Improve error message from logging.config.FileConfig#103628
vsajip merged 14 commits into
python:mainfrom
Agent-Hellboy:fix-issue-103606

Conversation

Agent-Hellboy commented Apr 19, 2023 •
edited by bedevere-bot
Loading

Copy link
Copy Markdown
Contributor

Agent-Hellboy requested a review from vsajip as a code owner April 19, 2023 12:04

bedevere-bot commented Apr 19, 2023

Copy link
Copy Markdown

Most changes to Python require a NEWS entry.

Please add it using the blurb_it web app or the blurb command-line tool.

bedevere-bot mentioned this pull request Apr 19, 2023
Closed
vsajip requested changes Apr 20, 2023
View reviewed changes

vsajip left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Choose a reason Spam Abuse Off Topic Outdated Duplicate Resolved Low Quality

Thanks for the PR.

Comment thread Lib/logging/config.py Outdated
Comment thread Lib/test/test_logging.py Outdated

bedevere-bot commented Apr 20, 2023

Copy link
Copy Markdown

A Python core developer has requested some changes be made to your pull request before we can consider merging it. If you could please address their requests along with any other requests in other reviews from core developers that would be appreciated.

Once you have made the requested changes, please leave a comment on this pull request containing the phrase I have made the requested changes; please review again. I will then notify any core developers who have left a review that you're ready for them to take another look at this pull request.

Agent-Hellboy commented Apr 20, 2023

Copy link
Copy Markdown
Contributor Author

Hi @vsajip I have made the requested changes; please review again,

one more question why doesn't the module has type annotation if this is because you lack people then I would like to do this as the module is very complex and interesting

bedevere-bot commented Apr 20, 2023

Copy link
Copy Markdown

Thanks for making the requested changes!

@vsajip: please review the changes made to this pull request.

bedevere-bot requested a review from vsajip April 20, 2023 09:51
Agent-Hellboy force-pushed the fix-issue-103606 branch 2 times, most recently from 3260bb8 to 64d7142 Compare April 20, 2023 17:05

Agent-Hellboy commented Apr 20, 2023

Copy link
Copy Markdown
Contributor Author

test on Windows is failing

ERROR: test_exception_if_confg_file_is_empty (test.test_logging.ConfigFileTest.test_exception_if_confg_file_is_empty)
----------------------------------------------------------------------
Traceback (most recent call last):
  File "D:\a\cpython\cpython\Lib\test\test_logging.py", line 1789, in test_exception_if_confg_file_is_empty
    os.unlink(fn)
PermissionError: [WinError 32] The process cannot access the file because it is being used by another process: 'C:\\Users\\RUNNER~1\\AppData\\Local\\Temp\\test_empty_eub_b3sw.ini'

is windows runner picking random runners in between?
previously I was removing the file in cleanup but now I am removing the file in the test itself but still, the test is failing

Agent-Hellboy commented Apr 20, 2023

Copy link
Copy Markdown
Contributor Author

test on Windows is failing

Passed

Agent-Hellboy commented May 12, 2023

Copy link
Copy Markdown
Contributor Author

Hi @vsajip, please review again

vsajip requested changes May 15, 2023
View reviewed changes
Comment thread Lib/logging/config.py Outdated
Comment thread Lib/logging/config.py Outdated
Comment thread Lib/logging/config.py Outdated
Comment thread Lib/test/test_logging.py Outdated
Comment thread Lib/test/test_logging.py Outdated

bedevere-bot commented May 15, 2023

Copy link
Copy Markdown

A Python core developer has requested some changes be made to your pull request before we can consider merging it. If you could please address their requests along with any other requests in other reviews from core developers that would be appreciated.

Once you have made the requested changes, please leave a comment on this pull request containing the phrase I have made the requested changes; please review again. I will then notify any core developers who have left a review that you're ready for them to take another look at this pull request.

And if you don't make the requested changes, you will be poked with soft cushions!

bedevere-bot commented May 16, 2023

Copy link
Copy Markdown

A Python core developer has requested some changes be made to your pull request before we can consider merging it. If you could please address their requests along with any other requests in other reviews from core developers that would be appreciated.

Once you have made the requested changes, please leave a comment on this pull request containing the phrase I have made the requested changes; please review again. I will then notify any core developers who have left a review that you're ready for them to take another look at this pull request.

Agent-Hellboy and others added 2 commits May 16, 2023 17:50
fix doc
340b860
Update Doc/library/logging.config.rst
5fb63b0 
Co-authored-by: Vinay Sajip <vinay_sajip@yahoo.co.uk>

Agent-Hellboy commented May 16, 2023

Copy link
Copy Markdown
Contributor Author

I have made the requested changes; please review again

bedevere-bot commented May 16, 2023

Copy link
Copy Markdown

Thanks for making the requested changes!

@vsajip: please review the changes made to this pull request.

bedevere-bot requested a review from vsajip May 16, 2023 12:33
vsajip added the 🔨 test-with-buildbots Test PR w/ buildbots; report in status section label May 17, 2023

bedevere-bot commented May 17, 2023

Copy link
Copy Markdown

🤖 New build scheduled with the buildbot fleet by @vsajip for commit 5fb63b0 🤖

If you want to schedule another build, you need to add the 🔨 test-with-buildbots label again.

bedevere-bot removed the 🔨 test-with-buildbots Test PR w/ buildbots; report in status section label May 17, 2023
vsajip merged commit 152227b into python:main May 18, 2023
vsajip added the needs backport to 3.11 only security fixes label May 18, 2023

miss-islington commented May 18, 2023

Copy link
Copy Markdown
Contributor

Thanks @Agent-Hellboy for the PR, and @vsajip for merging it 🌮🎉.. I'm working now to backport this PR to: 3.11.
🐍🍒⛏🤖

miss-islington commented May 18, 2023

Copy link
Copy Markdown
Contributor

Sorry, @Agent-Hellboy and @vsajip, I could not cleanly backport this to 3.11 due to a conflict.
Please backport using cherry_picker on command line.
cherry_picker 152227b569c3a9b87fe0483706f704762ced6d75 3.11

bedevere-bot commented May 18, 2023

Copy link
Copy Markdown

⚠️⚠️⚠️ Buildbot failure ⚠️⚠️⚠️

Hi! The buildbot aarch64 RHEL8 LTO + PGO 3.x has failed when building commit 152227b.

What do you need to do:

  1. Don't panic.
  2. Check the buildbot page in the devguide if you don't know what the buildbots are or how they work.
  3. Go to the page of the buildbot that failed (https://buildbot.python.org/all/#builders/78/builds/4412) and take a look at the build logs.
  4. Check if the failure is related to this commit (152227b) or if it is a false positive.
  5. If the failure is related to this commit, please, reflect that on the issue and make a new Pull Request with a fix.

You can take a look at the buildbot page here:

https://buildbot.python.org/all/#builders/78/builds/4412

Summary of the results of the build (if available):

Click to see traceback logs 
remote: Enumerating objects: 19, done.        
remote: Counting objects:   5% (1/19)        
remote: Counting objects:  10% (2/19)        
remote: Counting objects:  15% (3/19)        
remote: Counting objects:  21% (4/19)        
remote: Counting objects:  26% (5/19)        
remote: Counting objects:  31% (6/19)        
remote: Counting objects:  36% (7/19)        
remote: Counting objects:  42% (8/19)        
remote: Counting objects:  47% (9/19)        
remote: Counting objects:  52% (10/19)        
remote: Counting objects:  57% (11/19)        
remote: Counting objects:  63% (12/19)        
remote: Counting objects:  68% (13/19)        
remote: Counting objects:  73% (14/19)        
remote: Counting objects:  78% (15/19)        
remote: Counting objects:  84% (16/19)        
remote: Counting objects:  89% (17/19)        
remote: Counting objects:  94% (18/19)        
remote: Counting objects: 100% (19/19)        
remote: Counting objects: 100% (19/19), done.        
remote: Compressing objects:  10% (1/10)        
remote: Compressing objects:  20% (2/10)        
remote: Compressing objects:  30% (3/10)        
remote: Compressing objects:  40% (4/10)        
remote: Compressing objects:  50% (5/10)        
remote: Compressing objects:  60% (6/10)        
remote: Compressing objects:  70% (7/10)        
remote: Compressing objects:  80% (8/10)        
remote: Compressing objects:  90% (9/10)        
remote: Compressing objects: 100% (10/10)        
remote: Compressing objects: 100% (10/10), done.        
remote: Total 10 (delta 8), reused 1 (delta 0), pack-reused 0        
From https://github.com/python/cpython
 * branch                  main       -> FETCH_HEAD
Note: switching to '152227b569c3a9b87fe0483706f704762ced6d75'.

You are in 'detached HEAD' state. You can look around, make experimental
changes and commit them, and you can discard any commits you make in this
state without impacting any branches by switching back to a branch.

If you want to create a new branch to retain commits you create, you may
do so (now or later) by using -c with the switch command. Example:

  git switch -c <new-branch-name>

Or undo this operation with:

  git switch -

Turn off this advice by setting config variable advice.detachedHead to false

HEAD is now at 152227b569 gh-103606: Improve error message from logging.config.FileConfig (GH-103628)
Switched to and reset branch 'main'

find: ‘build’: No such file or directory
find: ‘build’: No such file or directory
find: ‘build’: No such file or directory
find: ‘build’: No such file or directory
make[2]: [Makefile:2770: clean-retain-profile] Error 1 (ignored)
Python/assemble.c: In function ‘_PyAssemble_MakeCodeObject’:
Python/assemble.c:130:9: warning: ‘handler.h_startdepth’ may be used uninitialized in this function [-Wmaybe-uninitialized]
     int depth = handler->h_startdepth - 1;
         ^~~~~
Python/assemble.c:147:34: note: ‘handler.h_startdepth’ was declared here
     _PyCompile_ExceptHandlerInfo handler;
                                  ^~~~~~~
./Modules/socketmodule.c: In function ‘sock_sendmsg_afalg’:
./Modules/socketmodule.c:8921:1: error: the control flow of function ‘sock_sendmsg_afalg’ does not match its profile data (counter ‘arcs’) [-Werror=coverage-mismatch]
 }
 ^
./Modules/socketmodule.c:8921:1: error: the control flow of function ‘sock_sendmsg_afalg’ does not match its profile data (counter ‘single’) [-Werror=coverage-mismatch]
./Modules/socketmodule.c:8921:1: error: the control flow of function ‘sock_sendmsg_afalg’ does not match its profile data (counter ‘average’) [-Werror=coverage-mismatch]
./Modules/socketmodule.c:8921:1: error: the control flow of function ‘sock_sendmsg_afalg’ does not match its profile data (counter ‘ior’) [-Werror=coverage-mismatch]
./Modules/socketmodule.c:8921:1: error: the control flow of function ‘sock_sendmsg_afalg’ does not match its profile data (counter ‘time_profiler’) [-Werror=coverage-mismatch]
./Modules/socketmodule.c: In function ‘sock_sendmsg’:
./Modules/socketmodule.c:8921:1: error: the control flow of function ‘sock_sendmsg’ does not match its profile data (counter ‘arcs’) [-Werror=coverage-mismatch]
./Modules/socketmodule.c:8921:1: error: the control flow of function ‘sock_sendmsg’ does not match its profile data (counter ‘single’) [-Werror=coverage-mismatch]
./Modules/socketmodule.c:8921:1: error: the control flow of function ‘sock_sendmsg’ does not match its profile data (counter ‘average’) [-Werror=coverage-mismatch]
./Modules/socketmodule.c:8921:1: error: the control flow of function ‘sock_sendmsg’ does not match its profile data (counter ‘ior’) [-Werror=coverage-mismatch]
./Modules/socketmodule.c:8921:1: error: the control flow of function ‘sock_sendmsg’ does not match its profile data (counter ‘time_profiler’) [-Werror=coverage-mismatch]
./Modules/socketmodule.c: In function ‘sock_recvmsg_guts’:
./Modules/socketmodule.c:8921:1: error: the control flow of function ‘sock_recvmsg_guts’ does not match its profile data (counter ‘arcs’) [-Werror=coverage-mismatch]
./Modules/socketmodule.c:8921:1: error: the control flow of function ‘sock_recvmsg_guts’ does not match its profile data (counter ‘single’) [-Werror=coverage-mismatch]
./Modules/socketmodule.c:8921:1: error: the control flow of function ‘sock_recvmsg_guts’ does not match its profile data (counter ‘indirect_call’) [-Werror=coverage-mismatch]
./Modules/socketmodule.c:8921:1: error: the control flow of function ‘sock_recvmsg_guts’ does not match its profile data (counter ‘average’) [-Werror=coverage-mismatch]
./Modules/socketmodule.c:8921:1: error: the control flow of function ‘sock_recvmsg_guts’ does not match its profile data (counter ‘ior’) [-Werror=coverage-mismatch]
./Modules/socketmodule.c:8921:1: error: the control flow of function ‘sock_recvmsg_guts’ does not match its profile data (counter ‘time_profiler’) [-Werror=coverage-mismatch]
cc1: some warnings being treated as errors
make[1]: *** [Makefile:3040: Modules/socketmodule.o] Error 1
make[1]: *** Waiting for unfinished jobs....
make: *** [Makefile:829: profile-opt] Error 2

bedevere-bot commented May 18, 2023

Copy link
Copy Markdown

⚠️⚠️⚠️ Buildbot failure ⚠️⚠️⚠️

Hi! The buildbot AMD64 Debian root 3.x has failed when building commit 152227b.

What do you need to do:

  1. Don't panic.
  2. Check the buildbot page in the devguide if you don't know what the buildbots are or how they work.
  3. Go to the page of the buildbot that failed (https://buildbot.python.org/all/#builders/345/builds/4821) and take a look at the build logs.
  4. Check if the failure is related to this commit (152227b) or if it is a false positive.
  5. If the failure is related to this commit, please, reflect that on the issue and make a new Pull Request with a fix.

You can take a look at the buildbot page here:

https://buildbot.python.org/all/#builders/345/builds/4821

Failed tests:

  • test_threading

Summary of the results of the build (if available):

==

Click to see traceback logs 
remote: Enumerating objects: 19, done.        
remote: Counting objects:   5% (1/19)        
remote: Counting objects:  10% (2/19)        
remote: Counting objects:  15% (3/19)        
remote: Counting objects:  21% (4/19)        
remote: Counting objects:  26% (5/19)        
remote: Counting objects:  31% (6/19)        
remote: Counting objects:  36% (7/19)        
remote: Counting objects:  42% (8/19)        
remote: Counting objects:  47% (9/19)        
remote: Counting objects:  52% (10/19)        
remote: Counting objects:  57% (11/19)        
remote: Counting objects:  63% (12/19)        
remote: Counting objects:  68% (13/19)        
remote: Counting objects:  73% (14/19)        
remote: Counting objects:  78% (15/19)        
remote: Counting objects:  84% (16/19)        
remote: Counting objects:  89% (17/19)        
remote: Counting objects:  94% (18/19)        
remote: Counting objects: 100% (19/19)        
remote: Counting objects: 100% (19/19), done.        
remote: Compressing objects:  10% (1/10)        
remote: Compressing objects:  20% (2/10)        
remote: Compressing objects:  30% (3/10)        
remote: Compressing objects:  40% (4/10)        
remote: Compressing objects:  50% (5/10)        
remote: Compressing objects:  60% (6/10)        
remote: Compressing objects:  70% (7/10)        
remote: Compressing objects:  80% (8/10)        
remote: Compressing objects:  90% (9/10)        
remote: Compressing objects: 100% (10/10)        
remote: Compressing objects: 100% (10/10), done.        
remote: Total 10 (delta 8), reused 1 (delta 0), pack-reused 0        
From https://github.com/python/cpython
 * branch                  main       -> FETCH_HEAD
Note: switching to '152227b569c3a9b87fe0483706f704762ced6d75'.

You are in 'detached HEAD' state. You can look around, make experimental
changes and commit them, and you can discard any commits you make in this
state without impacting any branches by switching back to a branch.

If you want to create a new branch to retain commits you create, you may
do so (now or later) by using -c with the switch command. Example:

  git switch -c <new-branch-name>

Or undo this operation with:

  git switch -

Turn off this advice by setting config variable advice.detachedHead to false

HEAD is now at 152227b569 gh-103606: Improve error message from logging.config.FileConfig (GH-103628)
Switched to and reset branch 'main'

configure: WARNING: pkg-config is missing. Some dependencies may not be detected correctly.

Fatal Python error: Segmentation fault

Thread 0x00007f924aa51280 (most recent call first):
  File "/root/buildarea/3.x.angelico-debian-amd64/build/Lib/unittest/runner.py", line 74 in _write_status
  File "/root/buildarea/3.x.angelico-debian-amd64/build/Lib/unittest/runner.py", line 95 in addSuccess
  File "/root/buildarea/3.x.angelico-debian-amd64/build/Lib/test/support/testresult.py", line 122 in addSuccess
  File "/root/buildarea/3.x.angelico-debian-amd64/build/Lib/unittest/case.py", line 648 in run
  File "/root/buildarea/3.x.angelico-debian-amd64/build/Lib/unittest/case.py", line 690 in __call__
  File "/root/buildarea/3.x.angelico-debian-amd64/build/Lib/unittest/suite.py", line 122 in run
  File "/root/buildarea/3.x.angelico-debian-amd64/build/Lib/unittest/suite.py", line 84 in __call__
  File "/root/buildarea/3.x.angelico-debian-amd64/build/Lib/unittest/suite.py", line 122 in run
  File "/root/buildarea/3.x.angelico-debian-amd64/build/Lib/unittest/suite.py", line 84 in __call__
  File "/root/buildarea/3.x.angelico-debian-amd64/build/Lib/unittest/suite.py", line 122 in run
  File "/root/buildarea/3.x.angelico-debian-amd64/build/Lib/unittest/suite.py", line 84 in __call__
  File "/root/buildarea/3.x.angelico-debian-amd64/build/Lib/unittest/runner.py", line 240 in run
  File "/root/buildarea/3.x.angelico-debian-amd64/build/Lib/test/support/__init__.py", line 1115 in _run_suite
  File "/root/buildarea/3.x.angelico-debian-amd64/build/Lib/test/support/__init__.py", line 1241 in run_unittest
  File "/root/buildarea/3.x.angelico-debian-amd64/build/Lib/test/libregrtest/runtest.py", line 294 in _test_module
  File "/root/buildarea/3.x.angelico-debian-amd64/build/Lib/test/libregrtest/runtest.py", line 330 in _runtest_inner2
  File "/root/buildarea/3.x.angelico-debian-amd64/build/Lib/test/libregrtest/runtest.py", line 373 in _runtest_inner
  File "/root/buildarea/3.x.angelico-debian-amd64/build/Lib/test/libregrtest/runtest.py", line 248 in _runtest
  File "/root/buildarea/3.x.angelico-debian-amd64/build/Lib/test/libregrtest/runtest.py", line 278 in runtest
  File "/root/buildarea/3.x.angelico-debian-amd64/build/Lib/test/libregrtest/main.py", line 365 in rerun_failed_tests
  File "/root/buildarea/3.x.angelico-debian-amd64/build/Lib/test/libregrtest/main.py", line 783 in _main
  File "/root/buildarea/3.x.angelico-debian-amd64/build/Lib/test/libregrtest/main.py", line 738 in main
  File "/root/buildarea/3.x.angelico-debian-amd64/build/Lib/test/libregrtest/main.py", line 802 in main
  File "/root/buildarea/3.x.angelico-debian-amd64/build/Lib/test/__main__.py", line 2 in <module>
  File "/root/buildarea/3.x.angelico-debian-amd64/build/Lib/runpy.py", line 88 in _run_code
  File "/root/buildarea/3.x.angelico-debian-amd64/build/Lib/runpy.py", line 198 in _run_module_as_main

Extension modules: _testcapi (total: 1)
Segmentation fault
make: *** [Makefile:1975: buildbottest] Error 139

Cannot open file '/root/buildarea/3.x.angelico-debian-amd64/build/test-results.xml' for upload

bedevere-bot commented May 20, 2023

Copy link
Copy Markdown

GH-104687 is a backport of this pull request to the 3.11 branch.

bedevere-bot removed the needs backport to 3.11 only security fixes label May 20, 2023

Agent-Hellboy commented May 20, 2023

Copy link
Copy Markdown
Contributor Author

Hi, @miss-islington I cherry-picked it on 3.11

Agent-Hellboy added a commit to Agent-Hellboy/cpython that referenced this pull request May 20, 2023
pythongh-103606: Improve error message from logging.config.FileConfig (…
0c1f304 
…pythonGH-103628)

(cherry picked from commit 152227b)
gpshead reviewed May 20, 2023
View reviewed changes
Comment thread Doc/library/logging.config.rst
.. versionadded:: 3.10
The *encoding* parameter is added.

.. versionadded:: 3.12

gpshead May 20, 2023

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Choose a reason Spam Abuse Off Topic Outdated Duplicate Resolved Low Quality

Strictly speaking I'd have said versionchanged rather than versionadded.

Comment thread Lib/logging/config.py
if not os.path.exists(fname):
raise FileNotFoundError(f"{fname} doesn't exist")
elif not os.path.getsize(fname):
raise ValueError(f'{fname} is an empty file')

gpshead May 20, 2023

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Choose a reason Spam Abuse Off Topic Outdated Duplicate Resolved Low Quality

ValueError is generally only used when the value itself is a problem, not when something external to the process such as a filesystem doesn't look right. Same below. If in doubt as to what exception to use here, I'd just go with RuntimeError if you don't have an Error exception class for your package already.

gpshead pushed a commit that referenced this pull request May 21, 2023
[3.11] gh-103606: Improve error message from logging.config.FileConfig (
2ade563 
GH-103628) (#104687)

* gh-103606: Improve error message from logging.config.FileConfig (GH-103628)

(cherry picked from commit 152227b)

plus backport the followup exception change fix to that in #104701
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters. Learn more about bidirectional Unicode characters
Show hidden characters
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

gpshead gpshead left review comments
vsajip vsajip approved these changes

Assignees

vsajip

Labels

skip news

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

5 participants

Back | FazBrowse Home | New Git URL