bpo-38361: syslog: fixed making default "ident" from sys.argv[0] #16557

vaclavbartos · 2019-10-03T11:38:23Z

The default value of "ident" parameter should be sys.argv[0] with leading path
components stripped, but it contained the last slash, i.e. '/program' instead
of 'program'.

BPO issue: https://bugs.python.org/issue38361

https://bugs.python.org/issue38361

Automerge-Triggered-By: @gvanrossum


        syslog: fixed making default "ident" from sys.argv[0]

The default value of "ident" parameter should be sys.argv[0] with leading path components stripped, but it contained the last slash, i.e. '/program' instead of 'program'.

the-knights-who-say-ni · 2019-10-03T11:38:26Z

Hello, and thanks for your contribution!

I'm a bot set up to make sure that the project can legally accept this contribution by verifying everyone involved has signed the PSF contributor agreement (CLA).

CLA Missing

Our records indicate the following people have not signed the CLA:

@vaclavbartos

For legal reasons we need all the people listed to sign the CLA before we can look at your contribution. Please follow the steps outlined in the CPython devguide to rectify this issue.

If you have recently signed the CLA, please wait at least one business day
before our records are updated.

You can check yourself to see if the CLA has been received.

Thanks again for the contribution, we look forward to reviewing it!

brandtbucher · 2019-10-03T16:01:02Z

Thanks for your time @vaclavbartos, and welcome to CPython! 😎

This looks like a good change. I assume you’ve already seen the bot’s message about the CLA?

brandtbucher

Even though this is a fairly trivial fix, I still think you should make a NEWS entry, since it changes logging output. Just something simple, like:

Fixed an issue where ``ident`` could include a leading path separator when :func:`syslog.openlog` was called without arguments.

And while a regression test or two are always good to include, it looks like this module has almost no test coverage currently due to platform issues. So, probably not necessary in this case.


        📜🤖 Added by blurb_it.

vaclavbartos · 2019-10-04T09:59:36Z

OK, I added the News entry. I signed the CLA yesterday and it's checked now.

Regarding tests - as you noted, it would be quite difficult to implement them, since there is currently no other way to read ident than to emit a syslog message, read it from a syslog file and parse ident from it - which is highly platfrom dependent.

brandtbucher · 2019-10-04T15:36:47Z

This module doesn't have any experts with GitHub accounts, and hasn't had any real changes in almost a decade. Given that this is a fairly trivial change, perhaps another core dev could look at it?

CC @vstinner

gvanrossum

Yeah, this looks good. I agree that this looks like a mis-translation of the original code, apparently introduced in revision d63a3b8.

gvanrossum · 2019-10-23T03:32:39Z

Should we backport this to 3.8? Then the behavior would be different in 3.8.1 than in 3.8.0.

brandtbucher · 2019-10-23T04:24:57Z

Even though it's a bug fix, I'm leaning "no". Logging output shouldn't change between minor versions, and I don't know of anyone who sees this as a must-have.

It's just another reason 3.9 will be better than 3.8.

brandtbucher · 2020-01-14T16:18:54Z

Going through some of my old reviews and saw that this was still unmerged. @gvanrossum, is this waiting for anything?

gvanrossum · 2020-01-14T17:57:23Z

Sorry! It should be merged now.

brandtbucher · 2020-01-14T17:58:21Z

Congrats on your first CPython contribution @vaclavbartos! 🍾

Looking forward to seeing more from you in the future.


        bpo-38361: syslog: fixed making default "ident" from sys.argv[0] (pyt…

…honGH-16557) The default value of "ident" parameter should be sys.argv[0] with leading path components stripped, but it contained the last slash, i.e. '/program' instead of 'program'. BPO issue: https://bugs.python.org/issue38361 https://bugs.python.org/issue38361


        bpo-38361: syslog: fixed making default "ident" from sys.argv[0] (pyt…

…honGH-16557) The default value of "ident" parameter should be sys.argv[0] with leading path components stripped, but it contained the last slash, i.e. '/program' instead of 'program'. BPO issue: https://bugs.python.org/issue38361 https://bugs.python.org/issue38361


        bpo-38361: syslog: fixed making default "ident" from sys.argv[0] (pyt…

…honGH-16557) The default value of "ident" parameter should be sys.argv[0] with leading path components stripped, but it contained the last slash, i.e. '/program' instead of 'program'. BPO issue: https://bugs.python.org/issue38361 https://bugs.python.org/issue38361

syslog: fixed making default "ident" from sys.argv[0]

Loading status checks…

73f8e9e

The default value of "ident" parameter should be sys.argv[0] with leading path components stripped, but it contained the last slash, i.e. '/program' instead of 'program'.

the-knights-who-say-ni added the CLA not signed label Oct 3, 2019

bedevere-bot added the awaiting review label Oct 3, 2019

vaclavbartos changed the title ~~bpo-NNNN: syslog: fixed making default "ident" from sys.argv[0]~~ bpo-38361: syslog: fixed making default "ident" from sys.argv[0] Oct 3, 2019

brandtbucher added the type-bugfix label Oct 3, 2019

brandtbucher suggested changes Oct 3, 2019

View changes

bedevere-bot added awaiting core review and removed awaiting review labels Oct 3, 2019

the-knights-who-say-ni added CLA signed and removed CLA not signed labels Oct 4, 2019

📜🤖 Added by blurb_it.

Verified

This commit was created on GitHub.com and signed with a verified signature using GitHub’s key.

GPG key ID: 4AEE18F83AFDEB23 Learn about signing commits

Loading status checks…

655944f

gvanrossum approved these changes Oct 23, 2019

View changes

bedevere-bot added awaiting merge and removed awaiting core review labels Oct 23, 2019

gvanrossum added the 🤖 automerge label Jan 14, 2020

bedevere-bot removed the awaiting merge label Jan 14, 2020

python / cpython

bpo-38361: syslog: fixed making default "ident" from sys.argv[0] #16557

bpo-38361: syslog: fixed making default "ident" from sys.argv[0] #16557

vaclavbartos commented Oct 3, 2019 •

edited by miss-islington

This comment has been minimized.

the-knights-who-say-ni commented Oct 3, 2019

This comment has been minimized.

brandtbucher commented Oct 3, 2019

brandtbucher left a comment

This comment has been minimized.

vaclavbartos commented Oct 4, 2019

This comment has been minimized.

brandtbucher commented Oct 4, 2019

gvanrossum left a comment

This comment has been minimized.

gvanrossum commented Oct 23, 2019

This comment has been minimized.

brandtbucher commented Oct 23, 2019

This comment has been minimized.

brandtbucher commented Jan 14, 2020

This comment has been minimized.

gvanrossum commented Jan 14, 2020

This comment has been minimized.

brandtbucher commented Jan 14, 2020

python / cpython

Sponsor python/cpython

Join GitHub today

bpo-38361: syslog: fixed making default "ident" from sys.argv[0] #16557

bpo-38361: syslog: fixed making default "ident" from sys.argv[0] #16557

Conversation

vaclavbartos commented Oct 3, 2019 • edited by miss-islington

This comment has been minimized.

the-knights-who-say-ni commented Oct 3, 2019

CLA Missing

This comment has been minimized.

brandtbucher commented Oct 3, 2019

brandtbucher left a comment

This comment has been minimized.

vaclavbartos commented Oct 4, 2019

This comment has been minimized.

brandtbucher commented Oct 4, 2019

gvanrossum left a comment

This comment has been minimized.

gvanrossum commented Oct 23, 2019

This comment has been minimized.

brandtbucher commented Oct 23, 2019

This comment has been minimized.

brandtbucher commented Jan 14, 2020

This comment has been minimized.

gvanrossum commented Jan 14, 2020

This comment has been minimized.

brandtbucher commented Jan 14, 2020

vaclavbartos commented Oct 3, 2019 •

edited by miss-islington