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/10/06 20:06:52 UTC

[GitHub] [airflow] frankcash opened a new pull request, #26925: [AIRFLOW-26916] Begin porting execution_date to logical_date - TriggerDagRunOperator

frankcash opened a new pull request, #26925:
URL: https://github.com/apache/airflow/pull/26925

   <!--
   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 an 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/
   -->
   
   ---
   **^ 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 changes, an Airflow Improvement Proposal ([AIP](https://cwiki.apache.org/confluence/display/AIRFLOW/Airflow+Improvement+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 a newsfragment file, named `{pr_number}.significant.rst` or `{issue_number}.significant.rst`, in [newsfragments](https://github.com/apache/airflow/tree/main/newsfragments).
   


-- 
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] frankcash commented on pull request #26925: TriggerDagRunOperator porting execution_date to logical_date

Posted by GitBox <gi...@apache.org>.
frankcash commented on PR #26925:
URL: https://github.com/apache/airflow/pull/26925#issuecomment-1273641916

   Looks like dask executor related tasks were having an issue, pulled from main since it looks like there is a 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] frankcash closed pull request #26925: TriggerDagRunOperator porting execution_date to logical_date

Posted by GitBox <gi...@apache.org>.
frankcash closed pull request #26925: TriggerDagRunOperator porting execution_date to logical_date
URL: https://github.com/apache/airflow/pull/26925


-- 
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 diff in pull request #26925: TriggerDagRunOperator porting execution_date to logical_date

Posted by GitBox <gi...@apache.org>.
uranusjr commented on code in PR #26925:
URL: https://github.com/apache/airflow/pull/26925#discussion_r991775913


##########
airflow/operators/trigger_dagrun.py:
##########
@@ -108,42 +111,49 @@ def __init__(
         self.allowed_states = allowed_states or [State.SUCCESS]
         self.failed_states = failed_states or [State.FAILED]
 
-        if execution_date is not None and not isinstance(execution_date, (str, datetime.datetime)):
+        if execution_date:
+            logical_date = execution_date
+            warnings.warn(
+                "Parameter ``execution_date`` is deprecated. Use ``logical_date``.",
+                RemovedInAirflow3Warning,
+            )

Review Comment:
   ```suggestion
               warnings.warn(
                   "Parameter ``execution_date`` is deprecated. Use ``logical_date``.",
                   RemovedInAirflow3Warning,
                   stacklevel=2
               )
   ```



##########
airflow/operators/trigger_dagrun.py:
##########
@@ -108,42 +111,49 @@ def __init__(
         self.allowed_states = allowed_states or [State.SUCCESS]
         self.failed_states = failed_states or [State.FAILED]
 
-        if execution_date is not None and not isinstance(execution_date, (str, datetime.datetime)):
+        if execution_date:
+            logical_date = execution_date
+            warnings.warn(
+                "Parameter ``execution_date`` is deprecated. Use ``logical_date``.",
+                RemovedInAirflow3Warning,
+            )

Review Comment:
   ```suggestion
               warnings.warn(
                   "Parameter ``execution_date`` is deprecated. Use ``logical_date``.",
                   RemovedInAirflow3Warning,
                   stacklevel=2,
               )
   ```



-- 
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 #26925: TriggerDagRunOperator porting execution_date to logical_date

Posted by GitBox <gi...@apache.org>.
potiuk commented on PR #26925:
URL: https://github.com/apache/airflow/pull/26925#issuecomment-1291286869

   And conflicts. 


-- 
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 #26925: [AIRFLOW-26916] Begin porting execution_date to logical_date - TriggerDagRunOperator

Posted by GitBox <gi...@apache.org>.
uranusjr commented on PR #26925:
URL: https://github.com/apache/airflow/pull/26925#issuecomment-1271114600

   We will need to leave to original `execution` field due to backward compatibility concerns. It can emit a deprecation warning when used, but must be kept.


-- 
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] iloveabm3 commented on pull request #26925: [AIRFLOW-26916] porting execution_date to logical_date - TriggerDagRunOperator

Posted by GitBox <gi...@apache.org>.
iloveabm3 commented on PR #26925:
URL: https://github.com/apache/airflow/pull/26925#issuecomment-1273442991

   frankcash:frankcash/AIRFLOW-26916


-- 
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 #26925: TriggerDagRunOperator porting execution_date to logical_date

Posted by GitBox <gi...@apache.org>.
potiuk commented on PR #26925:
URL: https://github.com/apache/airflow/pull/26925#issuecomment-1287940600

   I think some of the flaky tests need fixing here.


-- 
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 diff in pull request #26925: TriggerDagRunOperator porting execution_date to logical_date

Posted by GitBox <gi...@apache.org>.
uranusjr commented on code in PR #26925:
URL: https://github.com/apache/airflow/pull/26925#discussion_r991775589


##########
airflow/operators/trigger_dagrun.py:
##########
@@ -108,42 +111,49 @@ def __init__(
         self.allowed_states = allowed_states or [State.SUCCESS]
         self.failed_states = failed_states or [State.FAILED]
 
-        if execution_date is not None and not isinstance(execution_date, (str, datetime.datetime)):
+        if execution_date:
+            logical_date = execution_date
+            warnings.warn(
+                "Parameter ``execution_date`` is deprecated. Use ``logical_date``.",
+                RemovedInAirflow3Warning,
+            )
+
+        if logical_date is not None and not isinstance(logical_date, (str, datetime.datetime)):
             raise TypeError(
-                f"Expected str or datetime.datetime type for execution_date.Got {type(execution_date)}"
+                f"Expected str or datetime.datetime type for logical_date.Got {type(logical_date)}"

Review Comment:
   ```suggestion
                   f"Expected str or datetime.datetime type for logical_date. Got {type(logical_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] frankcash commented on pull request #26925: [AIRFLOW-26916] porting execution_date to logical_date - TriggerDagRunOperator

Posted by GitBox <gi...@apache.org>.
frankcash commented on PR #26925:
URL: https://github.com/apache/airflow/pull/26925#issuecomment-1273434617

   @uranusjr  / @eladkal 


-- 
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 #26925: TriggerDagRunOperator porting execution_date to logical_date

Posted by GitBox <gi...@apache.org>.
uranusjr commented on PR #26925:
URL: https://github.com/apache/airflow/pull/26925#issuecomment-1275856227

   Pushed another commit for that.


-- 
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] frankcash commented on pull request #26925: TriggerDagRunOperator porting execution_date to logical_date

Posted by GitBox <gi...@apache.org>.
frankcash commented on PR #26925:
URL: https://github.com/apache/airflow/pull/26925#issuecomment-1273806443

   @eladkal I believe this is ready for review now, not sure why the sqlite test is failing not getting anything from the log


-- 
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] frankcash commented on pull request #26925: [AIRFLOW-26916] Begin porting execution_date to logical_date - TriggerDagRunOperator

Posted by GitBox <gi...@apache.org>.
frankcash commented on PR #26925:
URL: https://github.com/apache/airflow/pull/26925#issuecomment-1271418869

   @uranusjr I believe this completes that field, I validated by creating a test function along the lines of
   
   `    td = TriggerDagRunOperator(task_id='td_os-get', trigger_dag_id='os-get-test', execution_date=datetime.datetime(2022, 4, 11, tzinfo=pytz.timezone('America/New_York')), wait_for_completion=True)` which properly worked and accepted `execution_date`, but just set the value as `logical_date` at the function init
   


-- 
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 diff in pull request #26925: TriggerDagRunOperator porting execution_date to logical_date

Posted by GitBox <gi...@apache.org>.
uranusjr commented on code in PR #26925:
URL: https://github.com/apache/airflow/pull/26925#discussion_r1005281880


##########
airflow/operators/trigger_dagrun.py:
##########
@@ -108,42 +111,66 @@ def __init__(
         self.allowed_states = allowed_states or [State.SUCCESS]
         self.failed_states = failed_states or [State.FAILED]
 
-        if execution_date is not None and not isinstance(execution_date, (str, datetime.datetime)):
-            raise TypeError(
-                f"Expected str or datetime.datetime type for execution_date.Got {type(execution_date)}"
-            )
-
-        self.execution_date = execution_date
+        if logical_date is None:
+            if execution_date is not None:
+                warnings.warn(
+                    "Parameter 'execution_date' is deprecated. Use 'logical_date' instead.",
+                    RemovedInAirflow3Warning,
+                    stacklevel=2,
+                )
+            logical_date = execution_date
+        elif not isinstance(logical_date, (str, datetime.datetime)):

Review Comment:
   This should be `if` so `execution_date` is also covered by this 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] frankcash commented on pull request #26925: TriggerDagRunOperator porting execution_date to logical_date

Posted by GitBox <gi...@apache.org>.
frankcash commented on PR #26925:
URL: https://github.com/apache/airflow/pull/26925#issuecomment-1310558790

   Unfortunately I am unable to focus on this right now 😐
   
   On Thu, Nov 10, 2022 at 11:26 AM Jarek Potiuk ***@***.***>
   wrote:
   
   > @frankcash <https://github.com/frankcash> - Just checking, are you stilll
   > working on it :) ?
   >
   > —
   > Reply to this email directly, view it on GitHub
   > <https://github.com/apache/airflow/pull/26925#issuecomment-1310556985>,
   > or unsubscribe
   > <https://github.com/notifications/unsubscribe-auth/ABN32R73WKHTWTBDY65JB2TWHUO4ZANCNFSM6AAAAAAQ663CFA>
   > .
   > You are receiving this because you were mentioned.Message ID:
   > ***@***.***>
   >
   -- 
   Charles Frank Cash
   FOSS Programmer
   ***@***.***
   https://github.com/frankcash
   https://keybase.io/frankcash
   <https://github.com/frankcash>
   


-- 
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 #26925: TriggerDagRunOperator porting execution_date to logical_date

Posted by GitBox <gi...@apache.org>.
potiuk commented on PR #26925:
URL: https://github.com/apache/airflow/pull/26925#issuecomment-1310556985

   @frankcash - Just checking, are you stilll working on 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] iloveabm3 commented on pull request #26925: [AIRFLOW-26916] porting execution_date to logical_date - TriggerDagRunOperator

Posted by GitBox <gi...@apache.org>.
iloveabm3 commented on PR #26925:
URL: https://github.com/apache/airflow/pull/26925#issuecomment-1273443724

   #26971 frankcash:frankcash/AIRFLOW-26916


-- 
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 #26925: TriggerDagRunOperator porting execution_date to logical_date

Posted by GitBox <gi...@apache.org>.
uranusjr commented on PR #26925:
URL: https://github.com/apache/airflow/pull/26925#issuecomment-1274031393

   > believe this keeps that field
   
   It does not keep the _field_, only the argument. You need to add a `@property` so access to `self.execution_date` (via subclass or wrapper) continues to work.


-- 
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 #26925: TriggerDagRunOperator porting execution_date to logical_date

Posted by GitBox <gi...@apache.org>.
eladkal commented on PR #26925:
URL: https://github.com/apache/airflow/pull/26925#issuecomment-1275815086

   Lets also update the tests https://github.com/apache/airflow/blob/main/tests/operators/test_trigger_dagrun.py
   ideally the tests should not reference deprecated parameters (unless it's to verify warning is raised)


-- 
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