Summary

Make review-requests API properly handle ambiguous times.

Review Request #8607 — Created Jan. 9, 2017 and submitted Jan. 13, 2017, 2:15 p.m.

Information

Owner

david

Repository

Review Board

Branch

release-2.5.x

Bugs

Depends On

Commit

c94f702...

Reviewers

Groups

reviewboard

People

Description

Because we live in a silly world where some people arbitrarily move their

clocks back and forth by an hour, timestamps, even those containing timezone

information, may not be unique. In particular, this could cause problems when

fetching the review-requests resource using the time-limiting fields with a

timestamp that exists during the hour after the transition from

daylight-savings time to standard time.
This change fixes things up to catch the error and return a reasonable error

via the API, rather than a 500 and a traceback. This also fixes it up so

invalid timestamp strings return an error rather than getting silently ignored.

Testing Done

Ran unit tests.

Issues

Description	From	Last Updated
local variable 'e' is assigned to but never used	reviewbot	Jan. 9, 2017, 9:34 p.m.
Any reason to do this over a loop?	brennie	Jan. 11, 2017, 3:15 p.m.
I was under the impression we went through django.utils.timezone (which I believe is a re-export of pytz?) throughout the rest …	brennie	Jan. 11, 2017, 3:15 p.m.
""" on next line.	brennie	Jan. 11, 2017, 3:16 p.m.

Tool: Pyflakes
Processed Files:
    reviewboard/webapi/resources/review_request.py
    reviewboard/webapi/tests/test_review_request.py



Tool: PEP8 Style Checker
Processed Files:
    reviewboard/webapi/resources/review_request.py
    reviewboard/webapi/tests/test_review_request.py

reviewboard/webapi/resources/review_request.py (Diff revision 1)
The issue has been resolved. Show all issues
```
 local variable 'e' is assigned to but never used
```

Commit:

ef7100e14107fd32bc3837aa925af64e3479bee3

37e114282d1445d41c2b85c8ec8f6d101672508f

Diff:

Revision 2 (+72 -31)

Show changes

	reviewboard/webapi/resources/review_request.py
	reviewboard/webapi/tests/test_review_request.py

Tool: Pyflakes
Processed Files:
    reviewboard/webapi/resources/review_request.py
    reviewboard/webapi/tests/test_review_request.py



Tool: PEP8 Style Checker
Processed Files:
    reviewboard/webapi/resources/review_request.py
    reviewboard/webapi/tests/test_review_request.py

Ship it!

reviewboard/webapi/resources/review_request.py (Diff revision 2)

The issue has been resolved. Show all issues

Any reason to do this over a loop?

reviewboard/webapi/tests/test_review_request.py (Diff revision 2)

The issue has been dropped. Show all issues

I was under the impression we went through django.utils.timezone (which I believe is a re-export of pytz?) throughout the rest of the codebase.

david Jan. 11, 2017, 3:16 p.m.

django.utils.timezone are django-specific things relating to activating a timezone and making datetimes aware.

reviewboard/webapi/tests/test_review_request.py (Diff revision 2)
The issue has been resolved. Show all issues
```
""" on next line.
```

Commit:

37e114282d1445d41c2b85c8ec8f6d101672508f

c94f702534a4a0fbfd319063bd945ef7d9f1be5b

Diff:

Revision 3 (+69 -31)

Show changes

	reviewboard/webapi/resources/review_request.py
	reviewboard/webapi/tests/test_review_request.py

Tool: Pyflakes
Processed Files:
    reviewboard/webapi/resources/review_request.py
    reviewboard/webapi/tests/test_review_request.py



Tool: PEP8 Style Checker
Processed Files:
    reviewboard/webapi/resources/review_request.py
    reviewboard/webapi/tests/test_review_request.py

Status:: Completed
Change Summary:: Pushed to release-2.5.x (b566240)