Summary

Localized timezone support (Djblets)

Review Request #2721 — Created Dec. 1, 2011 and submitted March 4, 2012, 11:05 a.m.

Information

Owner

ddruska

Repository

Djblets

Branch

Bugs

Depends On

Reviewers

Groups

djblets

People

Description

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.

Testing Done

Issues

Description	From	Last Updated
This isn't a built-in Python module, so make sure setup.py depends on it.	chipx86	Dec. 1, 2011, 3:42 p.m.
What happens on Django <= 1.3? Does this exist? Today, Djblets must work with Django 1.1 and higher, so you …	chipx86	Dec. 1, 2011, 3:42 p.m.
Same comments here.	chipx86	Dec. 1, 2011, 3:42 p.m.
Blank lines before and after this block.	chipx86	Dec. 1, 2011, 3:42 p.m.
Two blank lines.	chipx86	Dec. 1, 2011, 3:42 p.m.
"""A form blah blah""" First line should be summary, appended to """. If it can fit on one line, then …	chipx86	Dec. 1, 2011, 3:42 p.m.
Same question here.	chipx86	Dec. 1, 2011, 3:42 p.m.
Indent 4 spaces	mike_conley	Dec. 3, 2011, 1:01 a.m.
We'll probably want newlines before and after this block.	mike_conley	Dec. 3, 2011, 2:40 a.m.
We'll want a newline before this block.	mike_conley	Dec. 3, 2011, 2:40 a.m.

Fix it!

djblets/datagrid/grids.py (Diff revision 1)

An issue was opened. Show all issues

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

djblets/datagrid/grids.py (Diff revision 1)

An issue was opened. Show all issues

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.

djblets/util/fields.py (Diff revision 1)
An issue was opened. Show all issues
```
Same comments here.
```
djblets/util/fields.py (Diff revision 1)
An issue was opened. Show all issues
```
Blank lines before and after this block.
```
djblets/util/forms.py (Diff revision 1)
An issue was opened. Show all issues
```
Two blank lines.
```

djblets/util/forms.py (Diff revision 1)

An issue was opened. Show all issues

"""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.

djblets/util/templatetags/djblets_utils.py (Diff revision 1)
An issue was opened. Show all issues
```
Same question here.
```

Change Summary:

Added conditional imports

Diff:

Revision 2 (+72 -9)

Show changes

	setup.py
	djblets/datagrid/grids.py
	djblets/log/__init__.py
	djblets/util/fields.py
	djblets/util/forms.py
	djblets/util/tests.py
	djblets/util/templatetags/djblets_utils.py

```
Just found one further formatting nit.
```
djblets/datagrid/grids.py (Diff revision 2)
The issue has been resolved. Show all issues
```
Indent 4 spaces
```

Change Summary:

Fixed formatting error Mike found

Diff:

Revision 3 (+72 -9)

Show changes

	setup.py
	djblets/datagrid/grids.py
	djblets/log/__init__.py
	djblets/util/fields.py
	djblets/util/forms.py
	djblets/util/tests.py
	djblets/util/templatetags/djblets_utils.py

Ship it! Fix it!

Two final nits.  With those fixed, this gets my green light.

djblets/util/fields.py (Diff revision 3)

An issue was opened. Show all issues

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

djblets/util/templatetags/djblets_utils.py (Diff revision 3)
An issue was opened. Show all issues
```
We'll want a newline before this block.
```

Change Summary:

Added helper method for creating tz aware, UTC datetimes.
Fixed other stuff from reviews.

Diff:

Revision 4 (+78 -9)

Show changes

	setup.py
	djblets/datagrid/grids.py
	djblets/log/__init__.py
	djblets/util/fields.py
	djblets/util/forms.py
	djblets/util/tests.py
	djblets/util/templatetags/djblets_utils.py

Ship it!

```
Ship It!
```

Status:: Completed
Change Summary:: Pushed to master (76e1d03) with some small changes to just require Django 1.4.

Status:: Re-opened

Oops, Dave, you left out the change to dates.py :) Can you provide that?

Change Summary:

I think this is the only change required in dates.

Diff:

Revision 5 (+79 -10)

Show changes

	setup.py
	djblets/datagrid/grids.py
	djblets/log/__init__.py
	djblets/util/dates.py
	djblets/util/fields.py
	djblets/util/forms.py
	djblets/util/tests.py
	djblets/util/templatetags/djblets_utils.py

Ship it!

```
Looks good!
```

Status:: Completed
Change Summary:: Pushed to djblets/master as 687c3ab4bd

Status:: Re-opened

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

mike_conley Jan. 25, 2012, 3:33 p.m.

I've backed out the changeset, by the way.

Change Summary:

Got rid of NameError's and ImportError's, added dates.py to the commit.

Diff:

Revision 6 (+61 -10)

Show changes

	setup.py
	djblets/datagrid/grids.py
	djblets/log/__init__.py
	djblets/util/dates.py
	djblets/util/fields.py
	djblets/util/forms.py
	djblets/util/tests.py
	djblets/util/templatetags/djblets_utils.py

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?

chipx86 Jan. 26, 2012, 12:08 p.m.

I have a change pending to move to Django 1.4.

david Jan. 27, 2012, 8:36 a.m.

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.

chipx86 Feb. 4, 2012, 6:54 a.m.

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).

mike_conley March 3, 2012, 7:51 a.m.

Now that Djblets and Review Board both require Django 1.4, is this safe to land?

Ship it!

```
Ship It!
```

Status:: Completed
Change Summary:: Pushed to master (32faf1f). Thanks!