[GitHub] [airflow] Sonins commented on a change in pull request #20508: Fix dates.days_ago to respect localized timezone configuration.

[GitBox <gi...@apache.org>]

Sonins commented on a change in pull request #20508:
URL: https://github.com/apache/airflow/pull/20508#discussion_r776535016



##########
File path: tests/utils/test_dates.py
##########
@@ -29,8 +29,10 @@
 
 class TestDates(unittest.TestCase):

Review comment:
       Because dates.days_ago uses TIMEZONE in airflow.utils.timezone by `timezone.make_aware()`, I think patching `timezone.TIMEZONE`is more appropriate than `settings.TIMEZONE`. airflow.utils.timezone imports TIMEZONE by
   `from airflow.settings import TIMEZONE`
   
   But after patching `timezone.TIMEZONE` by direct assignment, test_round_time() is failed because of timezone difference by remaining patch. This is resolved by patching `timezone.TIMEZONE` by `with mock.patch()`. I'd not talk in detail about this problem. If you wonder, just let me know.
   
   But, There is problem in using utcnow() in days_ago. e.g. In 12/30 00:00 am in UTC, Los Angeles time is 12/29 16:00 pm. `dates.days_ago()` is expected to return 12/29 but it returns 12/30, although timezone is right (Los_Angeles).
   
   `timezone.make_aware()` does not seem to convert utcnow() time in `America/Los_Angeles` appropriately. Is this DST boundary problem you talked about?
   
   Anyway, I'm working on this problem right now. I'll comment as soon as this problem resolved.
   
   For now, test code will be
   
   ```python
   from unittest import mock
   
   class TestDates:
       @pytest.mark.parametrize('time_zone', ['UTC', 'America/Los_Angeles'])
       def test_days_ago(self, time_zone):
           with mock.patch('airflow.utils.timezone.TIMEZONE', pendulum.tz.timezone(time_zone)):
               today_midnight = pendulum.today(time_zone)
   
               assert today_midnight.tzinfo == dates.days_ago(2).tzinfo == pendulum.tz.timezone(time_zone)
               assert dates.days_ago(0) == today_midnight
               assert dates.days_ago(100) == today_midnight.add(days=-100) # For the DST boundary test.
               assert dates.days_ago(0, hour=3) == today_midnight + timedelta(hours=3)
               assert dates.days_ago(0, minute=3) == today_midnight + timedelta(minutes=3)
               assert dates.days_ago(0, second=3) == today_midnight + timedelta(seconds=3)
               assert dates.days_ago(0, microsecond=3) == today_midnight + timedelta(microseconds=3)
   
   ```
   
   




-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: commits-unsubscribe@airflow.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org