Tryton Code Review Tool
Help | Bug tracker | Discussion group | Source code | Sign in
(11)

Issue 423091003: tryton-env: Add timezone on cronjobs

Can't Edit
Can't Publish+Mail
Start Review
Created:
3 weeks, 1 day ago by nicoe
Modified:
1 week, 3 days ago
Reviewers:
pokoli, ced, reviewbot
Visibility:
Public.

Description

tryton-env: Add timezone on cronjobs issue9436

Patch Set 1 #

Total comments: 2

Patch Set 2 : Use zoneinfo or dateutil as timezone source and use a configuration entry #

Total comments: 13

Patch Set 3 : Add doc + better testing + use tz from environment #

Total comments: 12

Patch Set 4 : Add timezone in tools and timezone field to cron job #

Total comments: 38

Patch Set 5 : Fix remarks #

Total comments: 17

Patch Set 6 : Fix remarks #

Total comments: 14

Patch Set 7 : Fix remarks #

Patch Set 8 : Do tz conversion in next_call only #

Total comments: 1

Patch Set 9 : Adapt tests #

Patch Set 10 : Add UTC fallback when the environment variable is a non existing TZ #

Total comments: 4

Patch Set 11 : More compact handling of non existing IANA key #

Total comments: 7

Patch Set 12 : Remove duplication #

Patch Set 13 : Reuse active voice #

Total comments: 20

Patch Set 14 : Add test on UTC fallback #

Total comments: 1
Unified diffs Side-by-side diffs Delta from patch set Stats (+198 lines, -4 lines) Patch
M trytond/CHANGELOG View 1 2 3 4 1 chunk +1 line, -0 lines 0 comments Download
M trytond/doc/ref/tools/index.rst View 1 2 3 4 5 1 chunk +1 line, -0 lines 0 comments Download
A trytond/doc/ref/tools/timezone.rst View 1 2 3 4 5 6 12 1 chunk +25 lines, -0 lines 0 comments Download
M trytond/doc/topics/cron.rst View 1 2 3 4 5 6 1 chunk +5 lines, -0 lines 0 comments Download
M trytond/setup.py View 1 2 3 4 1 chunk +1 line, -0 lines 0 comments Download
M trytond/trytond/__init__.py View 1 2 3 4 1 chunk +1 line, -0 lines 0 comments Download
M trytond/trytond/ir/cron.py View 1 2 3 4 5 6 7 5 chunks +7 lines, -2 lines 0 comments Download
M trytond/trytond/ir/view/cron_form.xml View 2 3 1 chunk +2 lines, -0 lines 0 comments Download
M trytond/trytond/tests/test_ir.py View 1 2 3 4 5 6 7 8 3 chunks +103 lines, -1 line 0 comments Download
M trytond/trytond/tests/test_tools.py View 1 2 3 4 5 6 7 8 9 10 11 12 13 2 chunks +5 lines, -1 line 1 comment Download
A trytond/trytond/tools/timezone.py View 1 2 3 4 5 6 7 8 9 10 11 12 13 1 chunk +47 lines, -0 lines 0 comments Download

Messages

Total messages: 57
nicoe
3 weeks, 1 day ago (2022-05-04 16:43:58 UTC) #1
reviewbot
checks OK URL: https://codereview.tryton.org/423091003
3 weeks, 1 day ago (2022-05-04 16:54:36 UTC) #2
pokoli
https://codereview.tryton.org/423091003/diff/421091003/trytond/setup.py File trytond/setup.py (right): https://codereview.tryton.org/423091003/diff/421091003/trytond/setup.py#newcode75 trytond/setup.py:75: tests_require = ['pillow'] We should add pytz to tests ...
3 weeks, 1 day ago (2022-05-05 07:27:23 UTC) #3
nicoe
3 weeks, 1 day ago (2022-05-05 07:58:19 UTC) #4
nicoe
https://codereview.tryton.org/423091003/diff/417111003/trytond/trytond/ir/cron.py File trytond/trytond/ir/cron.py (right): https://codereview.tryton.org/423091003/diff/417111003/trytond/trytond/ir/cron.py#newcode28 trytond/trytond/ir/cron.py:28: _DISPLAY_TZ = config.get('database', 'timezone', default='UTC') I wonder if this ...
3 weeks, 1 day ago (2022-05-05 08:02:01 UTC) #5
reviewbot
ERROR: /tmp/reviewbot-2qbks07a/trytond/trytond/ir/cron.py Imports are incorrectly sorted and/or formatted. ERROR: /tmp/reviewbot-2qbks07a/trytond/trytond/tests/test_ir.py Imports are incorrectly sorted and/or ...
3 weeks, 1 day ago (2022-05-05 08:28:45 UTC) #6
ced
Missing documentation. https://codereview.tryton.org/423091003/diff/417111003/trytond/CHANGELOG File trytond/CHANGELOG (right): https://codereview.tryton.org/423091003/diff/417111003/trytond/CHANGELOG#newcode1 trytond/CHANGELOG:1: * Add timezone handling on cron jobs ...
3 weeks, 1 day ago (2022-05-05 12:28:37 UTC) #7
nicoe
https://codereview.tryton.org/423091003/diff/417111003/trytond/CHANGELOG File trytond/CHANGELOG (right): https://codereview.tryton.org/423091003/diff/417111003/trytond/CHANGELOG#newcode1 trytond/CHANGELOG:1: * Add timezone handling on cron jobs On 2022/05/05 ...
2 weeks, 3 days ago (2022-05-09 13:31:35 UTC) #8
nicoe
2 weeks, 3 days ago (2022-05-09 13:32:15 UTC) #9
reviewbot
ERROR: /tmp/reviewbot-jt9s8b6j/trytond/trytond/__init__.py Imports are incorrectly sorted and/or formatted. URL: https://codereview.tryton.org/423091003
2 weeks, 3 days ago (2022-05-09 13:58:07 UTC) #10
ced
https://codereview.tryton.org/423091003/diff/411131003/trytond/CHANGELOG File trytond/CHANGELOG (right): https://codereview.tryton.org/423091003/diff/411131003/trytond/CHANGELOG#newcode1 trytond/CHANGELOG:1: * Add timezone handling on cron jobs I would ...
2 weeks, 3 days ago (2022-05-09 21:02:39 UTC) #11
nicoe
https://codereview.tryton.org/423091003/diff/411131003/trytond/CHANGELOG File trytond/CHANGELOG (right): https://codereview.tryton.org/423091003/diff/411131003/trytond/CHANGELOG#newcode1 trytond/CHANGELOG:1: * Add timezone handling on cron jobs On 2022/05/09 ...
2 weeks, 2 days ago (2022-05-10 16:08:06 UTC) #12
nicoe
2 weeks, 2 days ago (2022-05-10 16:08:51 UTC) #13
reviewbot
checks OK URL: https://codereview.tryton.org/423091003
2 weeks, 2 days ago (2022-05-10 16:28:29 UTC) #14
ced
Missing tools documentation. https://codereview.tryton.org/423091003/diff/415101003/trytond/CHANGELOG File trytond/CHANGELOG (right): https://codereview.tryton.org/423091003/diff/415101003/trytond/CHANGELOG#newcode2 trytond/CHANGELOG:2: * Store local timezone setting in ...
2 weeks, 1 day ago (2022-05-11 21:55:52 UTC) #15
ced
https://codereview.tryton.org/423091003/diff/415101003/trytond/trytond/__init__.py File trytond/trytond/__init__.py (right): https://codereview.tryton.org/423091003/diff/415101003/trytond/trytond/__init__.py#newcode12 trytond/trytond/__init__.py:12: SERVER_TZKEY = os.environ.get('TZ', 'UTC') I think it should be: ...
2 weeks, 1 day ago (2022-05-12 06:40:59 UTC) #16
ced
https://codereview.tryton.org/423091003/diff/415101003/trytond/trytond/tools/timezone.py File trytond/trytond/tools/timezone.py (right): https://codereview.tryton.org/423091003/diff/415101003/trytond/trytond/tools/timezone.py#newcode14 trytond/trytond/tools/timezone.py:14: return sorted(zoneinfo.available_timezones()) I think we should fallback to pytz ...
2 weeks, 1 day ago (2022-05-12 06:53:45 UTC) #17
nicoe
https://codereview.tryton.org/423091003/diff/415101003/trytond/CHANGELOG File trytond/CHANGELOG (right): https://codereview.tryton.org/423091003/diff/415101003/trytond/CHANGELOG#newcode2 trytond/CHANGELOG:2: * Store local timezone setting in config On 2022/05/11 ...
1 week, 5 days ago (2022-05-14 21:50:25 UTC) #18
nicoe
1 week, 5 days ago (2022-05-14 21:50:49 UTC) #19
reviewbot
checks OK URL: https://codereview.tryton.org/423091003
1 week, 5 days ago (2022-05-14 22:18:30 UTC) #20
ced
https://codereview.tryton.org/423091003/diff/415101003/trytond/trytond/ir/cron.py File trytond/trytond/ir/cron.py (right): https://codereview.tryton.org/423091003/diff/415101003/trytond/trytond/ir/cron.py#newcode105 trytond/trytond/ir/cron.py:105: return {c.id: tz_key for c in crons} On 2022/05/14 ...
1 week, 5 days ago (2022-05-14 22:31:21 UTC) #21
nicoe
https://codereview.tryton.org/423091003/diff/433111003/trytond/doc/topics/cron.rst File trytond/doc/topics/cron.rst (right): https://codereview.tryton.org/423091003/diff/433111003/trytond/doc/topics/cron.rst#newcode18 trytond/doc/topics/cron.rst:18: If none of the variables is set it defaults ...
1 week, 4 days ago (2022-05-15 21:19:51 UTC) #22
nicoe
1 week, 4 days ago (2022-05-15 21:21:11 UTC) #23
reviewbot
checks OK URL: https://codereview.tryton.org/423091003
1 week, 4 days ago (2022-05-15 21:52:16 UTC) #24
ced
https://codereview.tryton.org/423091003/diff/433111003/trytond/trytond/ir/cron.py File trytond/trytond/ir/cron.py (right): https://codereview.tryton.org/423091003/diff/433111003/trytond/trytond/ir/cron.py#newcode70 trytond/trytond/ir/cron.py:70: timezone = fields.Function(fields.Char("Timezone"), 'get_timezone') On 2022/05/15 21:19:51, nicoe wrote: ...
1 week, 4 days ago (2022-05-16 06:29:39 UTC) #25
nicoe
https://codereview.tryton.org/423091003/diff/433111003/trytond/trytond/ir/cron.py File trytond/trytond/ir/cron.py (right): https://codereview.tryton.org/423091003/diff/433111003/trytond/trytond/ir/cron.py#newcode70 trytond/trytond/ir/cron.py:70: timezone = fields.Function(fields.Char("Timezone"), 'get_timezone') On 2022/05/16 06:29:39, ced wrote: ...
1 week, 4 days ago (2022-05-16 09:53:52 UTC) #26
nicoe
1 week, 4 days ago (2022-05-16 09:54:08 UTC) #27
ced
https://codereview.tryton.org/423091003/diff/433111003/trytond/trytond/ir/cron.py File trytond/trytond/ir/cron.py (right): https://codereview.tryton.org/423091003/diff/433111003/trytond/trytond/ir/cron.py#newcode157 trytond/trytond/ir/cron.py:157: now = datetime.datetime.now(tz=timezone.UTC) On 2022/05/16 09:53:51, nicoe wrote: > ...
1 week, 4 days ago (2022-05-16 10:14:10 UTC) #28
reviewbot
checks OK URL: https://codereview.tryton.org/423091003
1 week, 4 days ago (2022-05-16 10:17:49 UTC) #29
nicoe
https://codereview.tryton.org/423091003/diff/423231003/trytond/trytond/tools/timezone.py File trytond/trytond/tools/timezone.py (right): https://codereview.tryton.org/423091003/diff/423231003/trytond/trytond/tools/timezone.py#newcode28 trytond/trytond/tools/timezone.py:28: SERVER = ZoneInfo(os.environ['TRYTOND_TZ']) On 2022/05/16 10:14:10, ced wrote: > ...
1 week, 4 days ago (2022-05-16 11:32:59 UTC) #30
nicoe
1 week, 4 days ago (2022-05-16 11:36:44 UTC) #31
ced
https://codereview.tryton.org/423091003/diff/423231003/trytond/trytond/tools/timezone.py File trytond/trytond/tools/timezone.py (right): https://codereview.tryton.org/423091003/diff/423231003/trytond/trytond/tools/timezone.py#newcode28 trytond/trytond/tools/timezone.py:28: SERVER = ZoneInfo(os.environ['TRYTOND_TZ']) On 2022/05/16 11:32:59, nicoe wrote: > ...
1 week, 4 days ago (2022-05-16 11:49:49 UTC) #32
reviewbot
checks OK URL: https://codereview.tryton.org/423091003
1 week, 4 days ago (2022-05-16 11:55:41 UTC) #33
nicoe
1 week, 4 days ago (2022-05-16 12:03:17 UTC) #34
reviewbot
checks OK URL: https://codereview.tryton.org/423091003
1 week, 4 days ago (2022-05-16 12:17:15 UTC) #35
nicoe
1 week, 3 days ago (2022-05-16 13:34:21 UTC) #36
reviewbot
checks OK URL: https://codereview.tryton.org/423091003
1 week, 3 days ago (2022-05-16 13:53:51 UTC) #37
nicoe
https://codereview.tryton.org/423091003/diff/423231003/trytond/trytond/tools/timezone.py File trytond/trytond/tools/timezone.py (right): https://codereview.tryton.org/423091003/diff/423231003/trytond/trytond/tools/timezone.py#newcode28 trytond/trytond/tools/timezone.py:28: SERVER = ZoneInfo(os.environ['TRYTOND_TZ']) On 2022/05/16 11:49:49, ced wrote: > ...
1 week, 3 days ago (2022-05-16 13:58:09 UTC) #38
ced
https://codereview.tryton.org/423091003/diff/423231003/trytond/trytond/tools/timezone.py File trytond/trytond/tools/timezone.py (right): https://codereview.tryton.org/423091003/diff/423231003/trytond/trytond/tools/timezone.py#newcode28 trytond/trytond/tools/timezone.py:28: SERVER = ZoneInfo(os.environ['TRYTOND_TZ']) On 2022/05/16 13:58:08, nicoe wrote: > ...
1 week, 3 days ago (2022-05-16 14:17:30 UTC) #39
nicoe
https://codereview.tryton.org/423091003/diff/423231003/trytond/trytond/tools/timezone.py File trytond/trytond/tools/timezone.py (right): https://codereview.tryton.org/423091003/diff/423231003/trytond/trytond/tools/timezone.py#newcode28 trytond/trytond/tools/timezone.py:28: SERVER = ZoneInfo(os.environ['TRYTOND_TZ']) On 2022/05/16 14:17:30, ced wrote: > ...
1 week, 3 days ago (2022-05-16 15:30:53 UTC) #40
nicoe
1 week, 3 days ago (2022-05-16 15:31:39 UTC) #41
reviewbot
checks OK URL: https://codereview.tryton.org/423091003
1 week, 3 days ago (2022-05-16 15:51:48 UTC) #42
ced
https://codereview.tryton.org/423091003/diff/431131003/trytond/trytond/tools/timezone.py File trytond/trytond/tools/timezone.py (right): https://codereview.tryton.org/423091003/diff/431131003/trytond/trytond/tools/timezone.py#newcode39 trytond/trytond/tools/timezone.py:39: except zoneinfo.ZoneInfoNotFoundError: Will fail when zoneinfo is None. Really ...
1 week, 3 days ago (2022-05-16 16:26:11 UTC) #43
nicoe
https://codereview.tryton.org/423091003/diff/431131003/trytond/trytond/tools/timezone.py File trytond/trytond/tools/timezone.py (right): https://codereview.tryton.org/423091003/diff/431131003/trytond/trytond/tools/timezone.py#newcode39 trytond/trytond/tools/timezone.py:39: except zoneinfo.ZoneInfoNotFoundError: On 2022/05/16 16:26:11, ced wrote: > Will ...
1 week, 3 days ago (2022-05-16 19:23:57 UTC) #44
nicoe
1 week, 3 days ago (2022-05-16 19:24:07 UTC) #45
reviewbot
checks OK URL: https://codereview.tryton.org/423091003
1 week, 3 days ago (2022-05-16 19:46:55 UTC) #46
nicoe
1 week, 3 days ago (2022-05-16 20:24:08 UTC) #47
ced
https://codereview.tryton.org/423091003/diff/431131003/trytond/trytond/tools/timezone.py File trytond/trytond/tools/timezone.py (right): https://codereview.tryton.org/423091003/diff/431131003/trytond/trytond/tools/timezone.py#newcode39 trytond/trytond/tools/timezone.py:39: except zoneinfo.ZoneInfoNotFoundError: On 2022/05/16 19:23:57, nicoe wrote: > On ...
1 week, 3 days ago (2022-05-16 20:40:25 UTC) #48
reviewbot
checks OK URL: https://codereview.tryton.org/423091003
1 week, 3 days ago (2022-05-16 20:46:16 UTC) #49
nicoe
https://codereview.tryton.org/423091003/diff/413181004/trytond/trytond/tools/timezone.py File trytond/trytond/tools/timezone.py (right): https://codereview.tryton.org/423091003/diff/413181004/trytond/trytond/tools/timezone.py#newcode38 trytond/trytond/tools/timezone.py:38: if not zoneinfo and not SERVER: On 2022/05/16 20:40:25, ...
1 week, 3 days ago (2022-05-16 21:40:54 UTC) #50
ced
https://codereview.tryton.org/423091003/diff/413181004/trytond/doc/ref/tools/timezone.rst File trytond/doc/ref/tools/timezone.rst (right): https://codereview.tryton.org/423091003/diff/413181004/trytond/doc/ref/tools/timezone.rst#newcode7 trytond/doc/ref/tools/timezone.rst:7: .. class:: ZoneInfo(key) key is a strange name for ...
1 week, 3 days ago (2022-05-17 06:59:21 UTC) #51
nicoe
https://codereview.tryton.org/423091003/diff/431131003/trytond/trytond/tools/timezone.py File trytond/trytond/tools/timezone.py (right): https://codereview.tryton.org/423091003/diff/431131003/trytond/trytond/tools/timezone.py#newcode39 trytond/trytond/tools/timezone.py:39: except zoneinfo.ZoneInfoNotFoundError: On 2022/05/16 20:40:24, ced wrote: > On ...
1 week, 3 days ago (2022-05-17 07:40:18 UTC) #52
nicoe
1 week, 3 days ago (2022-05-17 07:41:20 UTC) #53
reviewbot
checks OK URL: https://codereview.tryton.org/423091003
1 week, 3 days ago (2022-05-17 07:49:58 UTC) #54
ced
https://codereview.tryton.org/423091003/diff/431131003/trytond/trytond/tools/timezone.py File trytond/trytond/tools/timezone.py (right): https://codereview.tryton.org/423091003/diff/431131003/trytond/trytond/tools/timezone.py#newcode39 trytond/trytond/tools/timezone.py:39: except zoneinfo.ZoneInfoNotFoundError: On 2022/05/17 07:40:18, nicoe wrote: > On ...
1 week, 3 days ago (2022-05-17 07:52:40 UTC) #55
nicoe
https://codereview.tryton.org/423091003/diff/413181004/trytond/doc/ref/tools/timezone.rst File trytond/doc/ref/tools/timezone.rst (right): https://codereview.tryton.org/423091003/diff/413181004/trytond/doc/ref/tools/timezone.rst#newcode7 trytond/doc/ref/tools/timezone.rst:7: .. class:: ZoneInfo(key) On 2022/05/17 06:59:21, ced wrote: > ...
1 week, 3 days ago (2022-05-17 08:19:41 UTC) #56
ced
1 week, 3 days ago (2022-05-17 08:36:03 UTC) #57
https://codereview.tryton.org/423091003/diff/413181004/trytond/doc/ref/tools/...
File trytond/doc/ref/tools/timezone.rst (right):

https://codereview.tryton.org/423091003/diff/413181004/trytond/doc/ref/tools/...
trytond/doc/ref/tools/timezone.rst:7: .. class:: ZoneInfo(key)
On 2022/05/17 08:19:40, nicoe wrote:
> On 2022/05/17 06:59:21, ced wrote:
> > key is a strange name for me zoneid describe better as it is how IANA call
it.
> 
> key is the name python use in their documentation.

Python may have choose a not so great name.
Also we make a tailored version so better to use name that suit us.

https://codereview.tryton.org/423091003/diff/413181004/trytond/trytond/tools/...
File trytond/trytond/tools/timezone.py (right):

https://codereview.tryton.org/423091003/diff/413181004/trytond/trytond/tools/...
trytond/trytond/tools/timezone.py:40: except ZoneInfoNotFoundError:
On 2022/05/17 08:19:41, nicoe wrote:
> On 2022/05/17 06:59:21, ced wrote:
> > On 2022/05/16 21:40:54, nicoe wrote:
> > > On 2022/05/16 20:40:24, ced wrote:
> > > > should catch KeyError if there is no TRYTOND_TZ in environ.
> > > 
> > > There will be TRYTOND_TZ in the environ, it's guaranteed by the code in
> > > trytond/__init__.py
> > 
> > No it is not. It is a global variable that can be manipulated by any body.
> 
> Just like any other object can be manipulated so you can not rely on any
> assumption.
> 
> If there is no reason to catch a KeyError when doing a pool.get, there is no
> reason to catch it here.

It is a startup code that should not fail, in contrary to where pool.get is used
inside a global catch all exception.
Sign in to reply to this message.

Powered by Google App Engine
RSS Feeds Recent Issues | This issue
This is Rietveld d9ca037-tainted