Edgewall Software
Modify

Opened 11 years ago

Closed 11 years ago

Last modified 5 years ago

#11286 closed enhancement (invalid)

Exception message attribute is deprecated in Python 2.6

Reported by: Ryan J Ollos Owned by: Ryan J Ollos
Priority: normal Milestone:
Component: general Version: 1.0-stable
Severity: normal Keywords: exception deprecated
Cc: Jun Omae Branch:
Release Notes:
API Changes:
Internal Changes:

Description

Pointed out in comment:8:ticket:11272 is that Exception's message attribute is deprecated in Python 2.6. See PEP:0352 for details.

The following uses will be replaced:

user@ubuntu:~/Workspace/trac-1.0-stable$ grep -R "\be.message\b" . --exclude=*.pot --exclude-dir=.svn
./trac/mimeview/api.py:                self.log.warning("Can't use annotator '%s': %s", a, e.message)
./trac/mimeview/api.py:                         annotator=tag.em(a), error=tag.pre(e.message))))
./trac/timeline/web_ui.py:                return tag.a(label, title=to_unicode(e.message),
./trac/versioncontrol/api.py:                                error=to_unicode(e.message)))
./trac/versioncontrol/api.py:                          error=to_unicode(e.message)))
./trac/versioncontrol/web_ui/browser.py:                raise ResourceNotFound(e.message,
./trac/versioncontrol/web_ui/changeset.py:            raise ResourceNotFound(e.message, _('Invalid Changeset Number'))
./trac/versioncontrol/web_ui/util.py:            tag.p(e.message, class_="message"),

Attachments (1)

hint-iso8601-for-timeline-link.diff (2.7 KB ) - added by Jun Omae 11 years ago.

Download all attachments as: .zip

Change History (24)

comment:1 by Ryan J Ollos, 11 years ago

Oh, I see now. The message property is defined on TracError, so it shouldn't matter that it is deprecated for BaseException. For instances of TracError it appears there is no problem accessing the message property.

Last edited 11 years ago by Ryan J Ollos (previous) (diff)

comment:2 by Ryan J Ollos, 11 years ago

Resolution: invalid
Status: newclosed

comment:3 by Ryan J Ollos, 11 years ago

In rjollos.git:t11286 I added test cases for the timeline module and refactored to use to_unicode(e) for consistency.

I also wonder if we should deprecate the message property of TracError for consistency with standard Python exceptions.

comment:4 by Ryan J Ollos, 11 years ago

Changes from rjollos.git:t11286 committed to 1.0-stable in [12008:12011] and merged to trunk in [12012:12014].

comment:5 by Ryan J Ollos, 11 years ago

There is a problem with [12009]. The unit tests pass (PYTHONPATH=. ./trac/test.py --skip-functional-tests), but when the functional tests are executed as well (PYTHONPATH=. ./trac/test.py), there is a test failure:

-<a class="timeline missing" title="&#34;2008-01-0A&#34; is an invalid date, or the date format is not known. Try &#34;MM/DD/YY&#34; instead.">timeline:2008-01-0A</a>
+<a class="timeline missing" title="&#34;2008-01-0A&#34; is an invalid date, or the date format is not known. Try &#34;MM/DD/YYYY&#34; instead.">timeline:2008-01-0A</a>

by Jun Omae, 11 years ago

in reply to:  5 ; comment:6 by Jun Omae, 11 years ago

Cc: Jun Omae added

Replying to rjollos:

There is a problem with [12009]. The unit tests pass (PYTHONPATH=. ./trac/test.py --skip-functional-tests), but when the functional tests are executed as well (PYTHONPATH=. ./trac/test.py), there is a test failure:

Reproduced.

The hint for date format is generated from datetime(1999, 10, 29).strftime('%x'). Also, the functional tests call setlocale in FunctionalTestEnvironment. If the unit and functional tests in the same run, the hint is changed.

Python 2.6.6 (r266:84292, Feb 22 2013, 00:00:18)
[GCC 4.4.7 20120313 (Red Hat 4.4.7-3)] on linux2
Type "help", "copyright", "credits" or "license" for more information.
>>> from datetime import datetime
>>> print datetime.now().strftime("%x")
09/06/13
>>> from locale import LC_ALL, setlocale
>>> setlocale(LC_ALL, "")
'en_US.UTF8'
>>> print datetime.now().strftime("%x")
09/06/2013

The timeline: link requires iso8601 format. If the date string is invalid, we should use YYYY-MM-DDThh:mm:ss±hh:mm instead of DD/MM/YY. See hint-iso8601-for-timeline-link.diff. Thoughts?

in reply to:  6 comment:7 by Ryan J Ollos, 11 years ago

Replying to jomae:

Also, the functional tests call setlocale in FunctionalTestEnvironment.

Okay, I see that happens here. I tried commenting out that line, leaving the session default LC_ALL='C' and expected to see the #11291 test failure prior to [12020], but did not see that test fail on Linux. There were a few test failures associated with the milestone admin module.

The patch looks good to me. I noticed while experimenting that date formats other than iso8601 are allowed, such as timeline:12/31/01. This is because parse_date tries various formats. We could restrict this by passing locale='iso8601' to parse_date, but that doesn't seem necessary. Perhaps the current approach is best - suggesting iso8601 in the documentation while allowing the other formats.

Last edited 11 years ago by Ryan J Ollos (previous) (diff)

comment:8 by Ryan J Ollos, 11 years ago

Milestone: 1.0.2

in reply to:  6 ; comment:9 by Ryan J Ollos, 11 years ago

Replying to jomae:

See hint-iso8601-for-timeline-link.diff. Thoughts?

Jun, did you want to do some more testing with the patch, or are you feeling that it is okay? LGTM. I'd be happy to push it to 1.0-stable, since an execution of all of the functional tests currently results in this failure.

in reply to:  9 ; comment:10 by Jun Omae, 11 years ago

Replying to rjollos:

Jun, did you want to do some more testing with the patch, or are you feeling that it is okay? LGTM.

Okay, the patch and unit tests have been pushed in [12087].

I'd be happy to push it to 1.0-stable, since an execution of all of the functional tests currently results in this failure.

Originally, if unit and functional tests are in the same run, we should execute the functional tests at the end? That might make the tests robust.

  • trac/test.py

    diff --git a/trac/test.py b/trac/test.py
    index 27e2873..4e5bbfa 100755
    a b def suite():  
    429429
    430430    suite = unittest.TestSuite()
    431431    suite.addTest(trac.tests.basicSuite())
    432     if INCLUDE_FUNCTIONAL_TESTS:
    433         suite.addTest(trac.tests.functionalSuite())
    434432    suite.addTest(trac.admin.tests.suite())
    435433    suite.addTest(trac.db.tests.suite())
    436434    suite.addTest(trac.mimeview.tests.suite())
    def suite():  
    446444    suite.addTest(tracopt.versioncontrol.git.tests.suite())
    447445    suite.addTest(tracopt.versioncontrol.svn.tests.suite())
    448446    suite.addTest(doctest.DocTestSuite(sys.modules[__name__]))
     447    if INCLUDE_FUNCTIONAL_TESTS:
     448        suite.addTest(trac.tests.functionalSuite())
    449449
    450450    return suite

in reply to:  10 ; comment:11 by Ryan J Ollos, 11 years ago

Replying to jomae:

Originally, if unit and functional tests are in the same run, we should execute the functional tests at the end? That might make the tests robust.

That change looks good to me, and all the tests still pass for me after making the change on 1.0-stable and trunk; Ubuntu 13.04, Python 2.6.

in reply to:  11 ; comment:12 by Jun Omae, 11 years ago

Replying to rjollos:

That change looks good to me, and all the tests still pass for me after making the change on 1.0-stable and trunk; Ubuntu 13.04, Python 2.6.

Thanks for your reviews! I've pushed in [12094] and merged into trunk in [12095].

in reply to:  6 ; comment:13 by Ryan J Ollos, 10 years ago

Replying to jomae:

The timeline: link requires iso8601 format. If the date string is invalid, we should use YYYY-MM-DDThh:mm:ss±hh:mm instead of DD/MM/YY. See hint-iso8601-for-timeline-link.diff. Thoughts?

I was looking at this again while reviewing your changes in #11342. With latest 1.0-stable and trunk, the timeline TracLink might allow formats other than ISO 8601, such as timeline:10/21/2013. That isn't the case for this Trac instance, but it is formatted as a followable TracLink in my development environment. I'm not understanding whether this is affected by the locale or the behavior has changed recently and we are just running an instance of Trac for t.e.o that is slightly behind the head of the trunk.

Does the following patch make sense?

  • trac/timeline/web_ui.py

    index 6e53d6b..cc54370 100644
    a b class TimelineModule(Component):  
    326326                elif len(time) >= 2:
    327327                    precision = 'hours'
    328328            try:
    329                 dt = parse_date(path, utc, hint='iso8601')
     329                dt = parse_date(path, utc, locale='iso8601', hint='iso8601')
    330330                return self.get_timeline_link(formatter.req, dt, label,
    331331                                              precision, query, fragment)
    332332            except TracError, e:

With that patch, parse_date would only parse ISO 8601 formats and relative formats such as timeline:yesterday.

in reply to:  13 ; comment:14 by Jun Omae, 10 years ago

Replying to rjollos:

With that patch, parse_date would only parse ISO 8601 formats and relative formats such as timeline:yesterday.

+1, LGTM.

in reply to:  14 comment:15 by Ryan J Ollos, 10 years ago

Replying to jomae:

Replying to rjollos:

With that patch, parse_date would only parse ISO 8601 formats and relative formats such as timeline:yesterday.

+1, LGTM.

Thanks, I'll add some unit tests and then commit the change. I didn't see your comment until just now, so I ended up digging deeper and trying to convince myself this was the right way to go. I'll paste my comment below anyway.


Replying to rjollos:

I'm not understanding whether this is affected by the locale …

It seems to be determined by the locale of the process from which tracd was started. Since the locale parameter of parse_date hasn't been specified, parse_date will try to parse the string to a time tuple using time.strptime, which seems to use the locale of the process when determining the locale's date representation. Therefore 10/31/2013 is successfully parsed in my environment with LANG=en_US.UTF, but won't be parsed for other locales.

>>> import time
>>> import locale
>>> from locale import LC_ALL
>>> locale.getlocale()
(None, None)
>>> locale.setlocale(LC_ALL, 'en_US.UTF8')
'en_US.UTF8'
>>> time.strptime('10/31/2013', '%x')
time.struct_time(tm_year=2013, tm_mon=10, tm_mday=31, tm_hour=0, tm_min=0, tm_sec=0, tm_wday=3, tm_yday=304, tm_isdst=-1)
>>> locale.setlocale(LC_ALL, 'en_GB.UTF8')
'en_GB.UTF8'
>>> time.strptime('10/31/2013', '%x')
Traceback (most recent call last):
  File "<stdin>", line 1, in <module>
  File "/usr/lib/python2.7/_strptime.py", line 467, in _strptime_time
    return _strptime(data_string, format)[0]
  File "/usr/lib/python2.7/_strptime.py", line 325, in _strptime
    (data_string, format))
ValueError: time data '10/31/2013' does not match format '%x'
>>> time.strptime('31/10/13', '%x')
time.struct_time(tm_year=2013, tm_mon=10, tm_mday=31, tm_hour=0, tm_min=0, tm_sec=0, tm_wday=3, tm_yday=304, tm_isdst=-1)

Forcing the locale to iso8601 therefore seems like the right way to go.

comment:16 by Ryan J Ollos, 10 years ago

I don't seem to have any success setting the locale for a unit test run. For example,

$ LANG=en_US.UTF8 python
>>> import locale
>>> locale.getlocale()
(None, None)

I would have expected getlocale() to return ('en_US', 'UTF-8').

Similarly, when I put a locale.getlocale() in my unit test and execute LC_ALL=en_US.UTF8 python -m trac.timeline.tests.wikisyntax, it returns (None, None).

in reply to:  16 comment:17 by Jun Omae, 10 years ago

I would have expected getlocale() to return ('en_US', 'UTF-8').

locale.setlocale(locale.LC_ALL, '') must be called before call of getlocale() to get the result. However, unit tests don't call setlocale() and only functional tests call setlocale() at functional/testenv.py. The trac.timeline.tests.wikisyntax is a unit test.

comment:18 by Ryan J Ollos, 10 years ago

Is it possible to change from the command line the locale under which the unit tests run? I was trying to write a unit test for timeline:10/31/2013, to see it fail when the locale is en_US-UTF8 and then put the fix in place to make it pass.

comment:19 by Ryan J Ollos, 10 years ago

… or is it the case that the unit tests won't be affected by the locale of the console because there is no setlocale call in the unit tests? That would seem to agree with what I'm seeing.

in reply to:  18 ; comment:20 by Jun Omae, 10 years ago

Replying to rjollos:

Is it possible to change from the command line the locale under which the unit tests run? I was trying to write a unit test for timeline:10/31/2013, to see it fail when the locale is en_US-UTF8 and then put the fix in place to make it pass.

I suppose impossible because locale id for setlocale and the result aren't portable for both Unix and Windows. Instead, we can use time.strftime('%x', (2013, 10, 24, ...)).

  • trac/timeline/tests/wikisyntax.py

    diff --git a/trac/timeline/tests/wikisyntax.py b/trac/timeline/tests/wikisyntax.py
    index 215ea0a..839c6bc 100644
    a b  
    1111# individuals. For the exact contribution history, see the revision
    1212# history and logs, available at http://trac.edgewall.org/log/.
    1313
     14import time
    1415import unittest
    1516
    1617from trac.timeline.web_ui import TimelineModule
    timeline:2008-01-29T15:48  
    2324timeline:2008-01-29T15:48Z
    2425timeline:2008-01-29T16:48+01
    2526timeline:2008-01-0A
     27timeline:@datestr_libc@
    2628------------------------------
    2729<p>
    2830<a class="timeline" href="/timeline?from=2008-01-29T00%3A00%3A00Z" title="See timeline at 2008-01-29T00:00:00Z">timeline:2008-01-29
    timeline:2008-01-0A  
    3739
    3840def suite():
    3941    suite = unittest.TestSuite()
    40     suite.addTest(formatter.suite(TIMELINE_TEST_CASES, file=__file__))
     42    datestr_libc = time.strftime('%x', (2013, 10, 24, 0, 0, 0, 0, 0, -1))
     43    suite.addTest(formatter.suite(TIMELINE_TEST_CASES.replace('@datestr_libc@',
     44                                                              datestr_libc),
     45                                  file=__file__))
    4146    return suite
    4247
    4348if __name__ == '__main__':

in reply to:  20 comment:21 by Ryan J Ollos, 10 years ago

Replying to jomae:

I suppose impossible because locale id for setlocale and the result aren't portable for both Unix and Windows. Instead, we can use time.strftime('%x', (2013, 10, 24, ...)).

Thanks, that looks good!

Changes committed to 1.0-stable in [12209] and merged to trunk in [12210].

in reply to:  12 comment:22 by Ryan J Ollos, 10 years ago

Replying to jomae:

Thanks for your reviews! I've pushed in [12094] and merged into trunk in [12095].

I have a minor addition to [12094]. Since the functional tests are executed last, the FunctionalTestEnvironment object isn't constructed until late in the test run, and therefore the SKIP message is printed in the middle of the test case output:

$ PYTHONPATH=. python -m trac.test
SKIP: validation of XHTML output in functional tests (no lxml installed)
SKIP: mimeview/tests/pygments (no pygments installed)
SKIP: utils/tests/datefmt.py (no pytz installed)
SKIP: utils/tests/datefmt.py (no babel installed)
SKIP: tracopt/perm/tests/authz_policy.py (no configobj installed)
SKIP: tracopt/versioncontrol/svn/tests/svn_fs.py (no svn bindings)
SKIP: versioncontrol/tests/functional.py (no svn bindings)
SKIP: RegressionTestTicket11176 (ConfigObj not installed)
SKIP: reST wiki tests (no docutils)
SKIP: RegressionTestTicket8976 (ConfigObj not installed)
..............................................................................................................................................................................................................................................................................................................................................................................................................................................................................................................................................................................................................................................................................................................................................................................................................................................................................................................................................................................................................................................................................................................................................................................................................................................................SKIP: fine-grained permission tests. ConfigObj not installed.
........................................................................................................................................................
----------------------------------------------------------------------
Ran 1366 tests in 212.903s

OK

Also, even prior to [12094], the SKIP: fine-grained permission tests. ConfigObj not installed. message is printed last, after the SKIP messages for the functional tests that are being skipped.

$ PYTHONPATH=. python -m trac.wiki.tests.functional
SKIP: validation of XHTML output in functional tests (no lxml installed)
SKIP: reST wiki tests (no docutils)
SKIP: RegressionTestTicket8976 (ConfigObj not installed)
SKIP: fine-grained permission tests. ConfigObj not installed.

I propose the following minor change:

  • trac/tests/functional/__init__.py

    diff --git a/trac/tests/functional/__init__.py b/trac/tests/functional/__init__.
    index 1a8b8b1..f2a713c 100755
    a b try:  
    8484except ImportError:
    8585    has_svn = False
    8686
     87try:
     88    from configobj import ConfigObj
     89except ImportError:
     90    ConfigObj = None
     91    print "SKIP: fine-grained permission tests. ConfigObj not installed."
     92
    8793from datetime import datetime, timedelta
    8894
    8995from trac.tests.contentgen import random_sentence, random_page, random_word, \
  • trac/tests/functional/testenv.py

    diff --git a/trac/tests/functional/testenv.py b/trac/tests/functional/testenv.py
    index 286a56c..3988910 100755
    a b class FunctionalTestEnvironment(object):  
    7777        time.sleep(0.1) # Avoid race condition on Windows
    7878        self.create()
    7979        locale.setlocale(locale.LC_ALL, '')
    80         if not ConfigObj:
    81             print "SKIP: fine-grained permission tests. " \
    82                   "ConfigObj not installed."
    8380
    8481    trac_src = '.'

The result is:

$ PYTHONPATH=. python -m trac.test
SKIP: validation of XHTML output in functional tests (no lxml installed)
SKIP: fine-grained permission tests. ConfigObj not installed.
SKIP: mimeview/tests/pygments (no pygments installed)
SKIP: utils/tests/datefmt.py (no pytz installed)
SKIP: utils/tests/datefmt.py (no babel installed)
SKIP: tracopt/perm/tests/authz_policy.py (no configobj installed)
SKIP: tracopt/versioncontrol/svn/tests/svn_fs.py (no svn bindings)
SKIP: versioncontrol/tests/functional.py (no svn bindings)
SKIP: RegressionTestTicket11176 (ConfigObj not installed)
SKIP: reST wiki tests (no docutils)
SKIP: RegressionTestTicket8976 (ConfigObj not installed)
......................................................................................................................................................................................................................................................................................................................................................................................................................................................................................................................................................................................................................................................................................................................................................................................................................................................................................................................................................................................................................................................................................................................................................................................................................................................................................................................................................................................................................
----------------------------------------------------------------------
Ran 1366 tests in 216.698s

OK
$ PYTHONPATH=. python -m trac.wiki.tests.functional
SKIP: validation of XHTML output in functional tests (no lxml installed)
SKIP: fine-grained permission tests. ConfigObj not installed.
SKIP: reST wiki tests (no docutils)
SKIP: RegressionTestTicket8976 (ConfigObj not installed)
........
----------------------------------------------------------------------
Ran 8 tests in 11.958s

OK

comment:23 by Ryan J Ollos, 10 years ago

Change from comment:22 committed to 1.0-stable in [12223] and merged to trunk in [12224].

Modify Ticket

Change Properties
Set your email in Preferences
Action
as closed The owner will remain Ryan J Ollos.
The resolution will be deleted. Next status will be 'reopened'.
to The owner will be changed from Ryan J Ollos to the specified user.

Add Comment


E-mail address and name can be saved in the Preferences .
 
Note: See TracTickets for help on using tickets.