Localized timezone support (Djblets)

Review Request #2721 — Created Dec. 1, 2011 and submitted

Information

Djblets

Reviewers

Supports the USE_TZ feature in Django dev by creating UTC, timezone aware datetime objects.

DateTimeColumn and DateTimeSinceColumn are updated to accept a new parameter "timezone", which converts the datetime object into that timezone. A TimeZoneField form field is created to make implementing a timezone ChoiceField simple. Lastly, timezones are created as UTC and are checked to be aware/naive for backwards compatibility.

 
Description From Last Updated

This isn't a built-in Python module, so make sure setup.py depends on it.

chipx86chipx86

What happens on Django <= 1.3? Does this exist? Today, Djblets must work with Django 1.1 and higher, so you …

chipx86chipx86

Same comments here.

chipx86chipx86

Blank lines before and after this block.

chipx86chipx86

Two blank lines.

chipx86chipx86

"""A form blah blah""" First line should be summary, appended to """. If it can fit on one line, then …

chipx86chipx86

Same question here.

chipx86chipx86

Indent 4 spaces

mike_conleymike_conley

We'll probably want newlines before and after this block.

mike_conleymike_conley

We'll want a newline before this block.

mike_conleymike_conley
chipx86
  1. 
      
  2. djblets/datagrid/grids.py (Diff revision 1)
     
     
    This isn't a built-in Python module, so make sure setup.py depends on it.
  3. djblets/datagrid/grids.py (Diff revision 1)
     
     
    What happens on Django <= 1.3? Does this exist? Today, Djblets must work with Django 1.1 and higher, so you may need to make things conditional here.
  4. djblets/util/fields.py (Diff revision 1)
     
     
    Same comments here.
  5. djblets/util/fields.py (Diff revision 1)
     
     
     
    Blank lines before and after this block.
  6. djblets/util/forms.py (Diff revision 1)
     
     
     
     
    Two blank lines.
  7. djblets/util/forms.py (Diff revision 1)
     
     
     
     
    """A form blah blah"""
    
    First line should be summary, appended to """. If it can fit on one line, then it should be a one-liner.
  8. Same question here.
  9. 
      
DD
mike_conley
  1. Just found one further formatting nit.
  2. djblets/datagrid/grids.py (Diff revision 2)
     
     
    Indent 4 spaces
  3. 
      
DD
mike_conley
  1. Two final nits.  With those fixed, this gets my green light.
  2. djblets/util/fields.py (Diff revision 3)
     
     
     
     
     
     
    We'll probably want newlines before and after this block.
  3. djblets/util/templatetags/djblets_utils.py (Diff revision 3)
     
     
     
     
     
     
    We'll want a newline before this block.
  4. 
      
DD
chipx86
  1. Ship It!
  2. 
      
DD
DD
chipx86
  1. Oops, Dave, you left out the change to dates.py :) Can you provide that?
  2. 
      
DD
mike_conley
  1. Looks good!
  2. 
      
DD
DD
mike_conley
  1. Dave:
    
    So I committed the patch to Djblets, and when trying to kick off the devserver, I was getting:
    
    Running dependency checks (set DEBUG=False to turn this off)...
    Traceback (most recent call last):
      File "./reviewboard/manage.py", line 174, in <module>
        check_dependencies()
      File "./reviewboard/manage.py", line 41, in check_dependencies
        from reviewboard.admin import checks
      File "/media/Projects/reviewboard/reviewboard/admin/checks.py", line 40, in <module>
        from djblets.siteconfig.models import SiteConfiguration
      File "/media/Projects/djblets/djblets/siteconfig/models.py", line 31, in <module>
        from djblets.util.fields import JSONField
      File "/media/Projects/djblets/djblets/util/fields.py", line 42, in <module>
        from djblets.util.dates import get_tz_aware_utcnow
    ImportError: cannot import name get_tz_aware_utcnow
    
    Looking at djblets.util.dates, it looks like there's a function missing.  Can you look into this please?
    
    Thanks,
    
    -Mike
    1. I've backed out the changeset, by the way.
  2. 
      
DD
david
  1. If I try to run this using Django 1.3, I get the following:
    
    Traceback (most recent call last):
      File "./reviewboard/manage.py", line 174, in <module>
        check_dependencies()
      File "./reviewboard/manage.py", line 41, in check_dependencies
        from reviewboard.admin import checks
      File "/home/david/Projects/reviewboard/reviewboard/admin/checks.py", line 40, in <module>
        from djblets.siteconfig.models import SiteConfiguration
      File "/usr/local/lib/python2.7/dist-packages/Djblets-0.7alpha0.dev-py2.7.egg/djblets/siteconfig/models.py", line 31, in <module>
        from djblets.util.fields import JSONField
      File "/usr/local/lib/python2.7/dist-packages/Djblets-0.7alpha0.dev-py2.7.egg/djblets/util/fields.py", line 36, in <module>
        from django.utils.timezone import is_aware
    ImportError: No module named timezone
    
    
    It looks like a previous version of your change caught ImportError to support this. Why did you remove it?
    1. I have a change pending to move to Django 1.4.
    2. Okay. In that case, I think the plan going forward is:
      (a) wait for django 1.4 to release
      (b) wait for Christian's change
      (c) push this and the associated RB change.
    3. For master, I think it's okay to just require the 1.4 alpha. 0.6.x will stay on Django 1.3, and we'll continue to do releases off that. Developers of RB 1.7 will just need to use the master branch of Djblets (which we tend to do anyway).
    4. Now that Djblets and Review Board both require Django 1.4, is this safe to land?
  2. 
      
david
  1. Ship It!
  2. 
      
DD
Review request changed

Status: Closed (submitted)

Change Summary:

Pushed to master (32faf1f). Thanks!
Loading...