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 2022/03/24 11:08:52 UTC
[GitHub] [airflow] lwyszomi opened a new pull request #22501: Remove coerce_datetime usage from GCSTimeSpanFileTransformOperator
lwyszomi opened a new pull request #22501:
URL: https://github.com/apache/airflow/pull/22501
Related to the issue with the compatibility with Airflow 2.1.+
---
**^ Add meaningful description above**
Read the **[Pull Request Guidelines](https://github.com/apache/airflow/blob/main/CONTRIBUTING.rst#pull-request-guidelines)** for more information.
In case of fundamental code change, Airflow Improvement Proposal ([AIP](https://cwiki.apache.org/confluence/display/AIRFLOW/Airflow+Improvements+Proposals)) is needed.
In case of a new dependency, check compliance with the [ASF 3rd Party License Policy](https://www.apache.org/legal/resolved.html#category-x).
In case of backwards incompatible changes please leave a note in [UPDATING.md](https://github.com/apache/airflow/blob/main/UPDATING.md).
--
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 a change in pull request #22501: Remove coerce_datetime usage from GCSTimeSpanFileTransformOperator
Posted by GitBox <gi...@apache.org>.
potiuk commented on a change in pull request #22501:
URL: https://github.com/apache/airflow/pull/22501#discussion_r834228206
##########
File path: airflow/providers/google/cloud/operators/gcs.py
##########
@@ -740,19 +740,19 @@ def execute(self, context: "Context") -> List[str]:
try:
timespan_start = context["data_interval_start"]
timespan_end = context["data_interval_end"]
- except KeyError: # Data interval context variables are only available in Airflow 2.2+
- timespan_start = timezone.coerce_datetime(context["execution_date"])
- timespan_end = timezone.coerce_datetime(context["dag"].following_schedule(timespan_start))
+ except KeyError:
+ timespan_start = context["execution_date"]
Review comment:
should not we actually check and convert "potential" dt.timetime to pendulum.DateTime here and below ? that's what corce_datetime did actually.
--
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 #22501: Remove coerce_datetime usage from GCSTimeSpanFileTransformOperator
Posted by GitBox <gi...@apache.org>.
uranusjr commented on a change in pull request #22501:
URL: https://github.com/apache/airflow/pull/22501#discussion_r834283787
##########
File path: airflow/providers/google/cloud/operators/gcs.py
##########
@@ -740,19 +740,19 @@ def execute(self, context: "Context") -> List[str]:
try:
timespan_start = context["data_interval_start"]
timespan_end = context["data_interval_end"]
- except KeyError: # Data interval context variables are only available in Airflow 2.2+
- timespan_start = timezone.coerce_datetime(context["execution_date"])
- timespan_end = timezone.coerce_datetime(context["dag"].following_schedule(timespan_start))
+ except KeyError:
+ timespan_start = context["execution_date"]
Review comment:
Simply something like
```python
following_execution_date = context["dag"].following_schedule(context["execution_date"])
if following_execution_date is None:
timespan_end: Optional[pendulum.DateTime] = None
else:
timespan_end = pendulum.instance(following_execution_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] potiuk merged pull request #22501: Remove coerce_datetime usage from GCSTimeSpanFileTransformOperator
Posted by GitBox <gi...@apache.org>.
potiuk merged pull request #22501:
URL: https://github.com/apache/airflow/pull/22501
--
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] lwyszomi commented on a change in pull request #22501: Remove coerce_datetime usage from GCSTimeSpanFileTransformOperator
Posted by GitBox <gi...@apache.org>.
lwyszomi commented on a change in pull request #22501:
URL: https://github.com/apache/airflow/pull/22501#discussion_r834328613
##########
File path: airflow/providers/google/cloud/operators/gcs.py
##########
@@ -740,19 +740,19 @@ def execute(self, context: "Context") -> List[str]:
try:
timespan_start = context["data_interval_start"]
timespan_end = context["data_interval_end"]
- except KeyError: # Data interval context variables are only available in Airflow 2.2+
- timespan_start = timezone.coerce_datetime(context["execution_date"])
- timespan_end = timezone.coerce_datetime(context["dag"].following_schedule(timespan_start))
+ except KeyError:
+ timespan_start = context["execution_date"]
Review comment:
@potiuk @uranusjr Done
--
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] lwyszomi commented on pull request #22501: Remove coerce_datetime usage from GCSTimeSpanFileTransformOperator
Posted by GitBox <gi...@apache.org>.
lwyszomi commented on pull request #22501:
URL: https://github.com/apache/airflow/pull/22501#issuecomment-1077508490
@potiuk PR should fix https://github.com/apache/airflow/pull/22499
--
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 a change in pull request #22501: Remove coerce_datetime usage from GCSTimeSpanFileTransformOperator
Posted by GitBox <gi...@apache.org>.
potiuk commented on a change in pull request #22501:
URL: https://github.com/apache/airflow/pull/22501#discussion_r834228797
##########
File path: airflow/providers/google/cloud/operators/gcs.py
##########
@@ -740,19 +740,19 @@ def execute(self, context: "Context") -> List[str]:
try:
timespan_start = context["data_interval_start"]
timespan_end = context["data_interval_end"]
- except KeyError: # Data interval context variables are only available in Airflow 2.2+
- timespan_start = timezone.coerce_datetime(context["execution_date"])
- timespan_end = timezone.coerce_datetime(context["dag"].following_schedule(timespan_start))
+ except KeyError:
+ timespan_start = context["execution_date"]
Review comment:
cc: @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] lwyszomi commented on a change in pull request #22501: Remove coerce_datetime usage from GCSTimeSpanFileTransformOperator
Posted by GitBox <gi...@apache.org>.
lwyszomi commented on a change in pull request #22501:
URL: https://github.com/apache/airflow/pull/22501#discussion_r834285836
##########
File path: airflow/providers/google/cloud/operators/gcs.py
##########
@@ -740,19 +740,19 @@ def execute(self, context: "Context") -> List[str]:
try:
timespan_start = context["data_interval_start"]
timespan_end = context["data_interval_end"]
- except KeyError: # Data interval context variables are only available in Airflow 2.2+
- timespan_start = timezone.coerce_datetime(context["execution_date"])
- timespan_end = timezone.coerce_datetime(context["dag"].following_schedule(timespan_start))
+ except KeyError:
+ timespan_start = context["execution_date"]
Review comment:
ok, make sense
--
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 #22501: Remove coerce_datetime usage from GCSTimeSpanFileTransformOperator
Posted by GitBox <gi...@apache.org>.
uranusjr commented on a change in pull request #22501:
URL: https://github.com/apache/airflow/pull/22501#discussion_r834246295
##########
File path: airflow/providers/google/cloud/operators/gcs.py
##########
@@ -740,19 +740,19 @@ def execute(self, context: "Context") -> List[str]:
try:
timespan_start = context["data_interval_start"]
timespan_end = context["data_interval_end"]
- except KeyError: # Data interval context variables are only available in Airflow 2.2+
- timespan_start = timezone.coerce_datetime(context["execution_date"])
- timespan_end = timezone.coerce_datetime(context["dag"].following_schedule(timespan_start))
+ except KeyError:
+ timespan_start = context["execution_date"]
Review comment:
Yes I think we should. Since `context["execution_date"]` can never be None, this can simply use `pendulum.instance`. The `following_schedule()` one below needs an additional check because it _can_ be None.
--
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] lwyszomi commented on a change in pull request #22501: Remove coerce_datetime usage from GCSTimeSpanFileTransformOperator
Posted by GitBox <gi...@apache.org>.
lwyszomi commented on a change in pull request #22501:
URL: https://github.com/apache/airflow/pull/22501#discussion_r834271294
##########
File path: airflow/providers/google/cloud/operators/gcs.py
##########
@@ -740,19 +740,19 @@ def execute(self, context: "Context") -> List[str]:
try:
timespan_start = context["data_interval_start"]
timespan_end = context["data_interval_end"]
- except KeyError: # Data interval context variables are only available in Airflow 2.2+
- timespan_start = timezone.coerce_datetime(context["execution_date"])
- timespan_end = timezone.coerce_datetime(context["dag"].following_schedule(timespan_start))
+ except KeyError:
+ timespan_start = context["execution_date"]
Review comment:
@uranusjr I will convert to the `pendulum` object. but regarding `following_schedule` we checking if the result is `None`, what additional check you have in mind?
--
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