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

Remove deprecated %z formatting from the default LOG_DATEFORMAT #1278

Merged
merged 1 commit into from Jun 4, 2015

Conversation

@curita
Copy link
Member

@curita curita commented Jun 3, 2015

Closes #1270

Not sure if the timezone is going to be missed from the logs (twisted logging had it) and I'm not entirely sure why windows is expanding it to contain non-ascii chars as reported in the issue, but '%z' time formatting is deprecated as stated here: https://docs.python.org/2/library/time.html#id2 [EDIT: I misread it, but is not listed in https://docs.python.org/2/library/time.html#time.strftime so it seems it's not officially supported], so it's probably safer to just remove it. '%Y-%m-%d %H:%M:%S' is the default datefmt for 'asctime' if there isn't one specified as well.

Note that this change just aims to have a sane default, since this is really a python logging issue. Users could still set LOG_DATEFORMAT to whatever they want, but they'll have to deal with the issues the python logging could arise.

@curita curita force-pushed the curita:remove-tz-aware-logformat branch from 61fb16a to 367ea81 Jun 3, 2015
@kmike
Copy link
Member

@kmike kmike commented Jun 3, 2015

@curita it is %Z which is deprecated (we use %z), and it is deprecated by email-related RFC2822 in favor of %z. These format specifiers are not deprecated by Python; "deprecated" means that non-numeric time zones (%Z) are deprecated for use in emails.

I second your question from #1270 (comment) - it is not clear why is %z formatted as %Z for @binbibi.

@kmike
Copy link
Member

@kmike kmike commented Jun 3, 2015

Aha, it seems '%z' is not officially supported in Python 2.x - it is even not in a list of possible options in 2.x docs. It is added to the list in 3.x docs, but I'm not sure it is supported even in 3.x. I think it is fine to remove it.

@curita
Copy link
Member Author

@curita curita commented Jun 3, 2015

Just updated the issue description too :) I misread '%Z' as '%z', but it does seem is not officially supported.

dangra added a commit that referenced this pull request Jun 4, 2015
Remove deprecated %z formatting from the default LOG_DATEFORMAT
@dangra dangra merged commit d9bcd48 into scrapy:master Jun 4, 2015
1 check passed
1 check passed
continuous-integration/travis-ci/pr The Travis CI build passed
Details
@kmike
Copy link
Member

@kmike kmike commented Jun 5, 2015

The problem with this change is that without timezone info timestamps in logs become ambiguous. I wonder if it is possible to either force UTC time in logs or somehow bring timezones back.

@nramirezuy
Copy link
Contributor

@nramirezuy nramirezuy commented Jun 5, 2015

If I understand correctly we can use formatter.converter = time.gmtime

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Linked issues

Successfully merging this pull request may close these issues.

None yet

4 participants