You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@airflow.apache.org by GitBox <gi...@apache.org> on 2021/12/27 01:34:27 UTC
[GitHub] [airflow] Sonins opened a new pull request #20508: Fix dates.days_ago to respect localized timezone configuration.
Sonins opened a new pull request #20508:
URL: https://github.com/apache/airflow/pull/20508
<!--
Thank you for contributing! Please make sure that your code changes
are covered with tests. And in case of new features or big changes
remember to adjust the documentation.
Feel free to ping committers for the review!
In case of existing issue, reference it using one of the following:
closes: #ISSUE
related: #ISSUE
How to write a good git commit message:
http://chris.beams.io/posts/git-commit/
-->
---
related: #17720
closes: #17720
There was problem that days_ago only uses utc time.
If dag is set start_date with days_ago, then dag is scheduled in utc time.
After fixing this, the dags using days_ago are scheduled in local time.
For example, the dag below in timezone `Asia/Seoul`
```python
from airflow import DAG
from airflow.operators.dummy import DummyOperator
from airflow.utils.dates import days_ago
with DAG(
dag_id = "test_days_ago_dag",
start_date=days_ago(7),
schedule_interval="0 2 * * *"
) as dag:
dumb = DummyOperator(task_id="test_dummy", dag=dag)
```
is scheduled like below.
--
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
[GitHub] [airflow] eladkal commented on pull request #20508: Fix dates.days_ago to respect localized timezone configuration.
Posted by GitBox <gi...@apache.org>.
eladkal commented on pull request #20508:
URL: https://github.com/apache/airflow/pull/20508#issuecomment-1042621737
> My thought is let's deprecate this function and not make changes to it in the meantime.
Usually we don't put effort in fixing bugs for deprecated features but if a user already did the work to fix a bug - why not accept it?
--
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
[GitHub] [airflow] Sonins commented on a change in pull request #20508: Fix dates.days_ago to respect localized timezone configuration.
Posted by GitBox <gi...@apache.org>.
Sonins commented on a change in pull request #20508:
URL: https://github.com/apache/airflow/pull/20508#discussion_r782609737
##########
File path: tests/utils/test_dates.py
##########
@@ -27,18 +28,19 @@
from airflow.utils import dates, timezone
-class TestDates(unittest.TestCase):
- def test_days_ago(self):
- today = pendulum.today()
- today_midnight = pendulum.instance(datetime.fromordinal(today.date().toordinal()))
+class TestDates:
+ @pytest.mark.parametrize('timezone', ['UTC', 'America/Los_Angeles'])
+ def test_days_ago(self, timezone):
+ with mock.patch('airflow.utils.timezone.TIMEZONE', pendulum.tz.timezone(timezone)):
+ today_midnight = pendulum.today(timezone)
- assert dates.days_ago(0) == today_midnight
- assert dates.days_ago(100) == today_midnight + timedelta(days=-100)
-
- 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)
+ assert today_midnight.tzinfo == dates.days_ago(2).tzinfo == pendulum.tz.timezone(timezone)
+ 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)
Review comment:
Those indentation is necessary because `dates.days_ago` uses `timezone.TIMEZONE`. So it needs to use patched `timezone.TIMEZONE` by `with mock.patch()` statement.
--
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
[GitHub] [airflow] potiuk commented on pull request #20508: Fix dates.days_ago to respect localized timezone configuration.
Posted by GitBox <gi...@apache.org>.
potiuk commented on pull request #20508:
URL: https://github.com/apache/airflow/pull/20508#issuecomment-1043452068
I would be against accepting the fix actually.
I think we should not "fix" things we discourage. That undermines the effort of discouraging it. Actually the sentence from @Sonins explains it very well:
> I realized days_ago has a problem after I opened this pr so i'm not using it anymore
This is precisely what we wanted to happen. We really others who want to use it this way come here are realize they should not use it. This is a win.
--
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
[GitHub] [airflow] dstandish commented on pull request #20508: Fix dates.days_ago to respect localized timezone configuration.
Posted by GitBox <gi...@apache.org>.
dstandish commented on pull request #20508:
URL: https://github.com/apache/airflow/pull/20508#issuecomment-1043170821
> Usually we don't put effort in fixing bugs for deprecated features but if a user already did the work to fix a bug - why not accept it?
Yeah that's a fair point @eladkal. I'll have to raise another vote (lazy consensus should be fine) overriding this part of the proposal:
> Until removal we should not change the behavior of the function (apart from the warnings).
And I have no problem with that.
Any objection @uranusjr?
--
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
[GitHub] [airflow] Sonins commented on pull request #20508: Fix dates.days_ago to respect localized timezone configuration.
Posted by GitBox <gi...@apache.org>.
Sonins commented on pull request #20508:
URL: https://github.com/apache/airflow/pull/20508#issuecomment-1043303110
> @Sonins how curial is this fix for you ? and why? thinking about it... even in your current Airflow version (whatever that may be) you should not use `days_ago`. We will deprecate it soon but regardless - what prevents you from simply not using it? Thus avoiding the problem all together.
`days_ago` has its simplicity even considering the implied problem you guys pointed out. Because of that, ([as Bas pointed out](https://lists.apache.org/thread/5l15l5qz5jzsbq589zjrxnlycgyjs120)) a lot of people may still uses it regardless its problem, but It has been problem that days_ago only run in timezone UTC. I thought some people felt uncomfortable about it, so I suggested this fix.
I don't understand 'curial' mean but that was why I open this pr. I realized `days_ago` has a problem after I opened this pr so i'm not using it anymore. But still fix is fix.
--
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
[GitHub] [airflow] dstandish edited a comment on pull request #20508: Fix dates.days_ago to respect localized timezone configuration.
Posted by GitBox <gi...@apache.org>.
dstandish edited a comment on pull request #20508:
URL: https://github.com/apache/airflow/pull/20508#issuecomment-1043181813
Although, @eladkal @uranusjr I do have one thing i'd like to confirm. If we change the timezone of `days_ago`, does that mean we will change the timezone of any dag that is using it, and therefore we'll change the time of day all those dags run? (assuming that they have changed default timezone to be non-UTC)
Because if that's true we probably shouldn't merge it.
--
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
[GitHub] [airflow] dstandish closed pull request #20508: Fix dates.days_ago to respect localized timezone configuration.
Posted by GitBox <gi...@apache.org>.
dstandish closed pull request #20508:
URL: https://github.com/apache/airflow/pull/20508
--
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
[GitHub] [airflow] eladkal commented on pull request #20508: Fix dates.days_ago to respect localized timezone configuration.
Posted by GitBox <gi...@apache.org>.
eladkal commented on pull request #20508:
URL: https://github.com/apache/airflow/pull/20508#issuecomment-1043269649
@Sonins how curial is this fix for you ? and why?
thinking about it... even in your current Airflow version (whatever that may be) you should not use `days_ago`. We will deprecate it soon but regardless - what prevents you from simply not using it? Thus avoiding the problem all together.
--
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
[GitHub] [airflow] eladkal commented on pull request #20508: Fix dates.days_ago to respect localized timezone configuration.
Posted by GitBox <gi...@apache.org>.
eladkal commented on pull request #20508:
URL: https://github.com/apache/airflow/pull/20508#issuecomment-1043471854
I'm Ok with closing.
No one is really blocked by this bug
--
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
[GitHub] [airflow] dstandish commented on pull request #20508: Fix dates.days_ago to respect localized timezone configuration.
Posted by GitBox <gi...@apache.org>.
dstandish commented on pull request #20508:
URL: https://github.com/apache/airflow/pull/20508#issuecomment-1026219183
I created a discussion thread ([see here](https://lists.apache.org/list?dev@airflow.apache.org:lte=1y:days_ago)) on the mailing list for this issue.
My thought is let's deprecate this function and not make changes to it in the meantime.
2 others agreed with deprecation in the discussion thread and none voiced disagreement.
What do you think @uranusjr, @Sonins ?
--
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
[GitHub] [airflow] Sonins commented on a change in pull request #20508: Fix dates.days_ago to respect localized timezone configuration.
Posted by GitBox <gi...@apache.org>.
Sonins commented on a change in pull request #20508:
URL: https://github.com/apache/airflow/pull/20508#discussion_r790619791
##########
File path: airflow/utils/dates.py
##########
@@ -257,7 +257,9 @@ def days_ago(n, hour=0, minute=0, second=0, microsecond=0):
Get a datetime object representing `n` days ago. By default the time is
set to midnight.
"""
- today = timezone.utcnow().replace(hour=hour, minute=minute, second=second, microsecond=microsecond)
+ today = datetime.now(timezone.TIMEZONE).replace(
+ hour=hour, minute=minute, second=second, microsecond=microsecond
+ )
Review comment:
> It’s documented to return the local date
My wrong, I was confused about this. `date.today()` does returns the system local date, but i think we still needs to respect timezone settings in airflow.cfg, which is `timezone.TIMEZONE`, not system local date.
--
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
[GitHub] [airflow] dstandish commented on pull request #20508: Fix dates.days_ago to respect localized timezone configuration.
Posted by GitBox <gi...@apache.org>.
dstandish commented on pull request #20508:
URL: https://github.com/apache/airflow/pull/20508#issuecomment-1043247650
i'm going to try to verify the behavior w.r.t. tz change in the next day or so.
--
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
[GitHub] [airflow] Sonins edited a comment on pull request #20508: Fix dates.days_ago to respect localized timezone configuration.
Posted by GitBox <gi...@apache.org>.
Sonins edited a comment on pull request #20508:
URL: https://github.com/apache/airflow/pull/20508#issuecomment-1043262523
> If we change the timezone of `days_ago`, does that mean we will change the timezone of any dag that is using it, and therefore we'll change the time of day all those dags run? (assuming that they have changed default timezone to be non-UTC)
For this, how about adding `days_ago` boolean option like below for backward compatibility?
```python
def days_ago(n, hour=0, minute=0, second=0, microsecond=0, local=False):
"""
Get a datetime object representing `n` days ago. By default the time is
set to midnight.
"""
if local:
return datetime.combine(
datetime.now(timezone.TIMEZONE) - timedelta(days=n),
time(hour, minute, second, microsecond, tzinfo=timezone.TIMEZONE),
)
else:
return datetime.combine(
datetime.utcnow() - timedelta(days=n),
time(hour, minute, second, microsecond),
)
```
It will be user's choice to change `days_ago` timezone in that way, adding new option `local=True` like below.
```python
days_ago(7, local=True)
```
--
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
[GitHub] [airflow] uranusjr commented on a change in pull request #20508: Fix dates.days_ago to respect localized timezone configuration.
Posted by GitBox <gi...@apache.org>.
uranusjr commented on a change in pull request #20508:
URL: https://github.com/apache/airflow/pull/20508#discussion_r779342952
##########
File path: tests/utils/test_dates.py
##########
@@ -29,8 +29,10 @@
class TestDates(unittest.TestCase):
def test_days_ago(self):
+ from airflow.settings import TIMEZONE
+
today = pendulum.today()
- today_midnight = pendulum.instance(datetime.fromordinal(today.date().toordinal()))
+ today_midnight = pendulum.instance(datetime.fromordinal(today.date().toordinal()), tz=TIMEZONE)
Review comment:
I think this has different meanings in some edge case because `pendulum.today()` has some weird semantics if the local time is at a different date from UTC (e.g. at 2am in UTC+3, UTC is still on the previous day).
--
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
[GitHub] [airflow] Sonins commented on pull request #20508: Fix dates.days_ago to respect localized timezone configuration.
Posted by GitBox <gi...@apache.org>.
Sonins commented on pull request #20508:
URL: https://github.com/apache/airflow/pull/20508#issuecomment-1002848780
@dstandish
OK. I committed new change. Can you check?
--
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
[GitHub] [airflow] Sonins commented on a change in pull request #20508: Fix dates.days_ago to respect localized timezone configuration.
Posted by GitBox <gi...@apache.org>.
Sonins commented on a change in pull request #20508:
URL: https://github.com/apache/airflow/pull/20508#discussion_r782609737
##########
File path: tests/utils/test_dates.py
##########
@@ -27,18 +28,19 @@
from airflow.utils import dates, timezone
-class TestDates(unittest.TestCase):
- def test_days_ago(self):
- today = pendulum.today()
- today_midnight = pendulum.instance(datetime.fromordinal(today.date().toordinal()))
+class TestDates:
+ @pytest.mark.parametrize('timezone', ['UTC', 'America/Los_Angeles'])
+ def test_days_ago(self, timezone):
+ with mock.patch('airflow.utils.timezone.TIMEZONE', pendulum.tz.timezone(timezone)):
+ today_midnight = pendulum.today(timezone)
- assert dates.days_ago(0) == today_midnight
- assert dates.days_ago(100) == today_midnight + timedelta(days=-100)
-
- 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)
+ assert today_midnight.tzinfo == dates.days_ago(2).tzinfo == pendulum.tz.timezone(timezone)
+ 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)
Review comment:
Those indentation is necessary because `dates.days_ago` uses `timezone.TIMEZONE`. So it needs to use patched `timezone.TIMEZONE` by `with mock.patch()` statement.
```python
# days_ago()
today = datetime.now(timezone.TIMEZONE).replace(
hour=hour, minute=minute, second=second, microsecond=microsecond
)
```
--
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
[GitHub] [airflow] dstandish edited a comment on pull request #20508: Fix dates.days_ago to respect localized timezone configuration.
Posted by GitBox <gi...@apache.org>.
dstandish edited a comment on pull request #20508:
URL: https://github.com/apache/airflow/pull/20508#issuecomment-1043170821
> Usually we don't put effort in fixing bugs for deprecated features but if a user already did the work to fix a bug - why not accept it?
Yeah that's a fair point @eladkal. I'll have to raise another vote (lazy consensus should be fine) overriding this part of the proposal:
> Until removal we should not change the behavior of the function (apart from the warnings).
And I have no problem with doing that.
Any objection @uranusjr?
--
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
[GitHub] [airflow] Sonins commented on pull request #20508: Fix dates.days_ago to respect localized timezone configuration.
Posted by GitBox <gi...@apache.org>.
Sonins commented on pull request #20508:
URL: https://github.com/apache/airflow/pull/20508#issuecomment-1043262523
> If we change the timezone of `days_ago`, does that mean we will change the timezone of any dag that is using it, and therefore we'll change the time of day all those dags run? (assuming that they have changed default timezone to be non-UTC)
For this, how about adding `days_ago` boolean option like below for backward compatibility?
```python
def days_ago(n, hour=0, minute=0, second=0, microsecond=0, local=False):
"""
Get a datetime object representing `n` days ago. By default the time is
set to midnight.
"""
if local:
return datetime.combine(
datetime.now(timezone.TIMEZONE) - timedelta(days=n),
time(hour, minute, second, microsecond, tzinfo=timezone.TIMEZONE),
)
else:
return datetime.combine(
datetime.utcnow() - timedelta(days=n),
time(hour, minute, second, microsecond),
)
```
--
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
[GitHub] [airflow] dstandish commented on pull request #20508: Fix dates.days_ago to respect localized timezone configuration.
Posted by GitBox <gi...@apache.org>.
dstandish commented on pull request #20508:
URL: https://github.com/apache/airflow/pull/20508#issuecomment-1043181813
Although, @eladkal @uranusjr I do have one thing i'd like to confirm. If we change the timezone of `days_ago`, does that mean we will change the timezone of any dag that is using it, and therefore we'll change the time of day all those dags run? (assuming that they have changed default timezone to be non-UTC)
--
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
[GitHub] [airflow] uranusjr commented on a change in pull request #20508: Fix dates.days_ago to respect localized timezone configuration.
Posted by GitBox <gi...@apache.org>.
uranusjr commented on a change in pull request #20508:
URL: https://github.com/apache/airflow/pull/20508#discussion_r780969367
##########
File path: airflow/utils/dates.py
##########
@@ -257,7 +257,9 @@ def days_ago(n, hour=0, minute=0, second=0, microsecond=0):
Get a datetime object representing `n` days ago. By default the time is
set to midnight.
"""
- today = timezone.utcnow().replace(hour=hour, minute=minute, second=second, microsecond=microsecond)
+ today = datetime.now(timezone.TIMEZONE).replace(
+ hour=hour, minute=minute, second=second, microsecond=microsecond
+ )
Review comment:
Or even
```python
def days_ago(n, hour=0, minute=0, second=0, microsecond=0):
return datetime.combine(
date.today() - timedelta(days=n),
time(hour, minute, second, microsecond, tzinfo=timezone.TIMEZONE),
)
```
--
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
[GitHub] [airflow] Sonins commented on a change in pull request #20508: Fix dates.days_ago to respect localized timezone configuration.
Posted by 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.
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
[GitHub] [airflow] uranusjr commented on a change in pull request #20508: Fix dates.days_ago to respect localized timezone configuration.
Posted by GitBox <gi...@apache.org>.
uranusjr commented on a change in pull request #20508:
URL: https://github.com/apache/airflow/pull/20508#discussion_r782649432
##########
File path: airflow/utils/dates.py
##########
@@ -257,7 +257,9 @@ def days_ago(n, hour=0, minute=0, second=0, microsecond=0):
Get a datetime object representing `n` days ago. By default the time is
set to midnight.
"""
- today = timezone.utcnow().replace(hour=hour, minute=minute, second=second, microsecond=microsecond)
+ today = datetime.now(timezone.TIMEZONE).replace(
+ hour=hour, minute=minute, second=second, microsecond=microsecond
+ )
Review comment:
> date.today() returns today in UTC.
It’s documented to return the local date, so if it really returns the UTC time’s date as you describe, there’s a serious bug in Python. But all those ultimately depend on `time.time()` anyway, so let’s use your version since you already made sure it works 🙂
--
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
[GitHub] [airflow] Sonins commented on pull request #20508: Fix dates.days_ago to respect localized timezone configuration.
Posted by GitBox <gi...@apache.org>.
Sonins commented on pull request #20508:
URL: https://github.com/apache/airflow/pull/20508#issuecomment-1026414890
> I created a discussion thread ([see here](https://lists.apache.org/list?dev@airflow.apache.org:lte=1y:days_ago)) on the mailing list for this issue.
>
> My thought is let's deprecate this function and not make changes to it in the meantime.
>
> 2 others agreed with deprecation in the discussion thread and none voiced disagreement.
>
> What do you think @uranusjr, @Sonins ?
I read the thread you gave.
So `days_ago` is not used frequently by other airflow users? I read that business value it produces is too little.
I think `days_ago` is nice function to easily use, but i agree that it implies some problem, too.
So i'm ok to deprecate it.
--
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
[GitHub] [airflow] dstandish edited a comment on pull request #20508: Fix dates.days_ago to respect localized timezone configuration.
Posted by GitBox <gi...@apache.org>.
dstandish edited a comment on pull request #20508:
URL: https://github.com/apache/airflow/pull/20508#issuecomment-1043181813
Although, @eladkal @uranusjr I do have one thing i'd like to confirm. If we change the timezone of `days_ago`, does that mean we will change the timezone of any dag that is using it, and therefore we'll change the time of day all those dags run? (assuming that they have changed default timezone to be non-UTC)
Because if that's true we probably shouldn't merge it because it would be breaking behavior. Unless we want to interpret it as a bug. wdyt?
--
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
[GitHub] [airflow] dstandish commented on pull request #20508: Fix dates.days_ago to respect localized timezone configuration.
Posted by GitBox <gi...@apache.org>.
dstandish commented on pull request #20508:
URL: https://github.com/apache/airflow/pull/20508#issuecomment-1043511168
> @Sonins thank you for the effort
Yes, I second that.
Bringing this issue to light helped us to make airflow a little confusing / frustrating for users.
--
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
[GitHub] [airflow] eladkal commented on pull request #20508: Fix dates.days_ago to respect localized timezone configuration.
Posted by GitBox <gi...@apache.org>.
eladkal commented on pull request #20508:
URL: https://github.com/apache/airflow/pull/20508#issuecomment-1043483134
@Sonins thank you for the effort
--
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
[GitHub] [airflow] dstandish commented on pull request #20508: Fix dates.days_ago to respect localized timezone configuration.
Posted by GitBox <gi...@apache.org>.
dstandish commented on pull request #20508:
URL: https://github.com/apache/airflow/pull/20508#issuecomment-1043474005
ok great -- closing
--
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
[GitHub] [airflow] dstandish commented on a change in pull request #20508: Fix dates.days_ago to respect localized timezone configuration.
Posted by GitBox <gi...@apache.org>.
dstandish commented on a change in pull request #20508:
URL: https://github.com/apache/airflow/pull/20508#discussion_r775653054
##########
File path: tests/utils/test_dates.py
##########
@@ -29,8 +29,10 @@
class TestDates(unittest.TestCase):
def test_days_ago(self):
+ from airflow.settings import TIMEZONE
Review comment:
your change to this test, it passes in the current codebase i.e. without your change, because by default TIMEZONE is UTC
i think you need to patch the configuration with `from tests.test_utils.config import conf_vars`. see other usages.
may want to parameterize with 2 timezones, one utc one not
##########
File path: tests/utils/test_dates.py
##########
@@ -29,8 +29,10 @@
class TestDates(unittest.TestCase):
def test_days_ago(self):
+ from airflow.settings import TIMEZONE
+
today = pendulum.today()
- today_midnight = pendulum.instance(datetime.fromordinal(today.date().toordinal()))
+ today_midnight = pendulum.instance(datetime.fromordinal(today.date().toordinal()), tz=TIMEZONE)
assert dates.days_ago(0) == today_midnight
assert dates.days_ago(100) == today_midnight + timedelta(days=-100)
Review comment:
with the code in `days_ago` this will actually fail if you cross DST / PST boundary, because timedelta gives 24-hour period so it will be off by 1 hour. so e.g. it should fail right now for timezone `America/Los_Angeles`.
but i think if we make this change, we should change the code to subtract calendar days instead of timedelta
##########
File path: tests/utils/test_dates.py
##########
@@ -29,8 +29,10 @@
class TestDates(unittest.TestCase):
def test_days_ago(self):
+ from airflow.settings import TIMEZONE
Review comment:
hmm i tried patching default timezone and have not had luck (see https://github.com/apache/airflow/pull/20529)
but you should be able to do so like this (in a local import for example):
```
from airflow import settings
settings.TIMEZONE = pendulum.tz.timezone('America/Los_Angeles')
```
##########
File path: tests/utils/test_dates.py
##########
@@ -29,8 +29,10 @@
class TestDates(unittest.TestCase):
Review comment:
i think these tests should pass
```python
class TestDates:
@pytest.mark.parametrize('timezone', ['UTC', 'America/Los_Angeles'])
def test_days_ago(self, timezone):
from airflow import settings
settings.TIMEZONE = pendulum.tz.timezone(timezone)
from airflow.utils import dates
today_midnight = pendulum.today(settings.TIMEZONE)
assert today_midnight.tzinfo == dates.days_ago(2).tzinfo == pendulum.tz.timezone(timezone)
assert dates.days_ago(0) == today_midnight
assert dates.days_ago(100) == today_midnight.add(days=-100)
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)
```
you will need to add more logic to `days_ago` though.
additionally, i think rather than using moving time `pendulum.today` we should actually use a fixed time such as 2021-12-28 so that way we ensure that days_ago(100) actually does cross a DST boundary. i would also add a comment inline above the days_ago(100) assert that explains why we need it (namely to test behavior across DST boundary)
##########
File path: tests/utils/test_dates.py
##########
@@ -29,8 +29,10 @@
class TestDates(unittest.TestCase):
def test_days_ago(self):
+ from airflow.settings import TIMEZONE
+
today = pendulum.today()
- today_midnight = pendulum.instance(datetime.fromordinal(today.date().toordinal()))
+ today_midnight = pendulum.instance(datetime.fromordinal(today.date().toordinal()), tz=TIMEZONE)
Review comment:
can you not simply do
```
today_midnight = pendulum.today(TIMEZONE)
```
--
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
[GitHub] [airflow] github-actions[bot] commented on pull request #20508: Fix dates.days_ago to respect localized timezone configuration.
Posted by GitBox <gi...@apache.org>.
github-actions[bot] commented on pull request #20508:
URL: https://github.com/apache/airflow/pull/20508#issuecomment-1043176636
The PR most likely needs to run full matrix of tests because it modifies parts of the core of Airflow. However, committers might decide to merge it quickly and take the risk. If they don't merge it quickly - please rebase it to the latest main at your convenience, or amend the last commit of the PR, and push it with --force-with-lease.
--
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
[GitHub] [airflow] Sonins commented on a change in pull request #20508: Fix dates.days_ago to respect localized timezone configuration.
Posted by GitBox <gi...@apache.org>.
Sonins commented on a change in pull request #20508:
URL: https://github.com/apache/airflow/pull/20508#discussion_r776548184
##########
File path: tests/utils/test_dates.py
##########
@@ -29,8 +29,10 @@
class TestDates(unittest.TestCase):
Review comment:
Instead of using `datetime.utcnow()`, I just replace it with `datetime.now(timezone.TIMEZONE)`
So dates.days_ago code will be like below.
```python
def days_ago(n, hour=0, minute=0, second=0, microsecond=0):
"""
Get a datetime object representing `n` days ago. By default the time is
set to midnight.
"""
today = datetime.now(timezone.TIMEZONE).replace(
hour=hour, minute=minute, second=second, microsecond=microsecond
)
return today - timedelta(days=n)
```
For DST offset problem, I tested it with several condition and it looks fine with timedelta substraction.
test code:
```python
from datetime import datetime, timedelta
import pendulum
after_dst_end = "2021-11-08"
dst_timezone = pendulum.tz.timezone("America/Los_Angeles")
dt = datetime.fromisoformat(after_dst_end).replace(tzinfo=dst_timezone)
print(dt)
print(dt - timedelta(days=1))
print(dt - timedelta(days=2))
print(dt - timedelta(days=260))
```
result:
```
2021-11-08 00:00:00-08:00 # dt
2021-11-07 00:00:00-07:00 # dt - 1
2021-11-06 00:00:00-07:00 # dt - 2
2021-02-21 00:00:00-08:00 # dt - 260
```
--
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
[GitHub] [airflow] uranusjr commented on a change in pull request #20508: Fix dates.days_ago to respect localized timezone configuration.
Posted by GitBox <gi...@apache.org>.
uranusjr commented on a change in pull request #20508:
URL: https://github.com/apache/airflow/pull/20508#discussion_r780968724
##########
File path: airflow/utils/dates.py
##########
@@ -257,7 +257,9 @@ def days_ago(n, hour=0, minute=0, second=0, microsecond=0):
Get a datetime object representing `n` days ago. By default the time is
set to midnight.
"""
- today = timezone.utcnow().replace(hour=hour, minute=minute, second=second, microsecond=microsecond)
+ today = datetime.now(timezone.TIMEZONE).replace(
+ hour=hour, minute=minute, second=second, microsecond=microsecond
+ )
Review comment:
Maybe
```python
datetime.combine(
date.today(),
time(hour, minute, second, microsecond, tzinfo=timezone.TIMEZONE),
)
```
is easier to understand?
--
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
[GitHub] [airflow] uranusjr commented on a change in pull request #20508: Fix dates.days_ago to respect localized timezone configuration.
Posted by GitBox <gi...@apache.org>.
uranusjr commented on a change in pull request #20508:
URL: https://github.com/apache/airflow/pull/20508#discussion_r780969808
##########
File path: tests/utils/test_dates.py
##########
@@ -27,18 +28,19 @@
from airflow.utils import dates, timezone
-class TestDates(unittest.TestCase):
- def test_days_ago(self):
- today = pendulum.today()
- today_midnight = pendulum.instance(datetime.fromordinal(today.date().toordinal()))
+class TestDates:
+ @pytest.mark.parametrize('timezone', ['UTC', 'America/Los_Angeles'])
+ def test_days_ago(self, timezone):
+ with mock.patch('airflow.utils.timezone.TIMEZONE', pendulum.tz.timezone(timezone)):
+ today_midnight = pendulum.today(timezone)
- assert dates.days_ago(0) == today_midnight
- assert dates.days_ago(100) == today_midnight + timedelta(days=-100)
-
- 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)
+ assert today_midnight.tzinfo == dates.days_ago(2).tzinfo == pendulum.tz.timezone(timezone)
+ 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)
Review comment:
These indentation changes don’t seem to be necessary?
--
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
[GitHub] [airflow] Sonins commented on a change in pull request #20508: Fix dates.days_ago to respect localized timezone configuration.
Posted by GitBox <gi...@apache.org>.
Sonins commented on a change in pull request #20508:
URL: https://github.com/apache/airflow/pull/20508#discussion_r782623943
##########
File path: airflow/utils/dates.py
##########
@@ -257,7 +257,9 @@ def days_ago(n, hour=0, minute=0, second=0, microsecond=0):
Get a datetime object representing `n` days ago. By default the time is
set to midnight.
"""
- today = timezone.utcnow().replace(hour=hour, minute=minute, second=second, microsecond=microsecond)
+ today = datetime.now(timezone.TIMEZONE).replace(
+ hour=hour, minute=minute, second=second, microsecond=microsecond
+ )
Review comment:
`date.today()` returns today in UTC. So e.g. when it is `2021-01-12 00:00` in UTC, date.today() returns `2021-01-12` even though it is still `2021-01-11` in Los Angeles time. `today()` is classmethod and does not take any other argument, so we can just use `datetime.now()` for date parameter in `datetime.combine()`.
```python
def days_ago(n, hour=0, minute=0, second=0, microsecond=0):
return datetime.combine(
datetime.now(timezone.TIMEZONE) - timedelta(days=n),
time(hour, minute, second, microsecond, tzinfo=timezone.TIMEZONE),
)
```
--
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
[GitHub] [airflow] Sonins commented on a change in pull request #20508: Fix dates.days_ago to respect localized timezone configuration.
Posted by GitBox <gi...@apache.org>.
Sonins commented on a change in pull request #20508:
URL: https://github.com/apache/airflow/pull/20508#discussion_r782623943
##########
File path: airflow/utils/dates.py
##########
@@ -257,7 +257,9 @@ def days_ago(n, hour=0, minute=0, second=0, microsecond=0):
Get a datetime object representing `n` days ago. By default the time is
set to midnight.
"""
- today = timezone.utcnow().replace(hour=hour, minute=minute, second=second, microsecond=microsecond)
+ today = datetime.now(timezone.TIMEZONE).replace(
+ hour=hour, minute=minute, second=second, microsecond=microsecond
+ )
Review comment:
`date.today()` is UTC today(). So e.g. when it is `2021-01-12 00:00` in UTC, date.today() returns `2021-01-12` even though it is still `2021-01-11` in Los Angeles time. `today()` is classmethod and does not take any other argument, so we can just use `datetime.now()` for date parameter in `datetime.combine()`.
```python
def days_ago(n, hour=0, minute=0, second=0, microsecond=0):
return datetime.combine(
datetime.now(timezone.TIMEZONE) - timedelta(days=n),
time(hour, minute, second, microsecond, tzinfo=timezone.TIMEZONE),
)
```
--
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
[GitHub] [airflow] Sonins commented on a change in pull request #20508: Fix dates.days_ago to respect localized timezone configuration.
Posted by GitBox <gi...@apache.org>.
Sonins commented on a change in pull request #20508:
URL: https://github.com/apache/airflow/pull/20508#discussion_r790619791
##########
File path: airflow/utils/dates.py
##########
@@ -257,7 +257,9 @@ def days_ago(n, hour=0, minute=0, second=0, microsecond=0):
Get a datetime object representing `n` days ago. By default the time is
set to midnight.
"""
- today = timezone.utcnow().replace(hour=hour, minute=minute, second=second, microsecond=microsecond)
+ today = datetime.now(timezone.TIMEZONE).replace(
+ hour=hour, minute=minute, second=second, microsecond=microsecond
+ )
Review comment:
> It’s documented to return the local date
My wrong, I was confused about this. `date.today()` does returns the system local date. But i think we still needs to respect timezone settings in airflow.cfg, which is `timezone.TIMEZONE`, not system local date.
--
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
[GitHub] [airflow] uranusjr commented on pull request #20508: Fix dates.days_ago to respect localized timezone configuration.
Posted by GitBox <gi...@apache.org>.
uranusjr commented on pull request #20508:
URL: https://github.com/apache/airflow/pull/20508#issuecomment-1026471377
Cross-posting my response on the mailing list in a shorter form, the problem of `days_ago` is not (at least not entirely) that it’s not frequently used, but _the fact of it being used_ provides little advantage and ironically is a minor foot gun that almost never does what you intend to. Yes, even your usage of `start_date=days_ago(7)` is strongly discouraged because the expression is too ambiguous and leads to confusing behaviour down the road.
--
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
[GitHub] [airflow] Sonins commented on pull request #20508: Fix dates.days_ago to respect localized timezone configuration.
Posted by GitBox <gi...@apache.org>.
Sonins commented on pull request #20508:
URL: https://github.com/apache/airflow/pull/20508#issuecomment-1045980255
@eladkal @dstandish No problem 🍺
--
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
[GitHub] [airflow] Sonins commented on a change in pull request #20508: Fix dates.days_ago to respect localized timezone configuration.
Posted by 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