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/02/24 16:26:05 UTC

[GitHub] [airflow] Dev232001 opened a new pull request #21796: Replaced all days_ago functions with datetime functions

Dev232001 opened a new pull request #21796:
URL: https://github.com/apache/airflow/pull/21796


   related: #21683 
   Replace all the days_ago functions with datetime functions as asked in the issue
   
   


-- 
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] josh-fell commented on a change in pull request #21796: Replaced all days_ago functions with datetime functions

Posted by GitBox <gi...@apache.org>.
josh-fell commented on a change in pull request #21796:
URL: https://github.com/apache/airflow/pull/21796#discussion_r814104115



##########
File path: tests/api_connexion/endpoints/test_extra_link_endpoint.py
##########
@@ -95,7 +94,7 @@ def _create_dag():
         with DAG(
             dag_id="TEST_DAG_ID",
             default_args=dict(
-                start_date=days_ago(2),
+                start_date=datetime.datetime(2022, 1, 1),

Review comment:
       For unit tests as well as example DAGs for consistency in tests, examples, docs, etc. The core example DAGs have been transitioned but the provider ones haven't yet.




-- 
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] Dev232001 commented on pull request #21796: Replaced all days_ago functions with datetime functions

Posted by GitBox <gi...@apache.org>.
Dev232001 commented on pull request #21796:
URL: https://github.com/apache/airflow/pull/21796#issuecomment-1055031344


   Sure, Thanks
   
   On Tue, Mar 1, 2022, 7:03 AM Tzu-ping Chung ***@***.***>
   wrote:
   
   > Please fix the static check failures.
   >
   > β€”
   > Reply to this email directly, view it on GitHub
   > <https://github.com/apache/airflow/pull/21796#issuecomment-1054886715>,
   > or unsubscribe
   > <https://github.com/notifications/unsubscribe-auth/AXEWNKDPYBCSGSCMKS544FTU5VXXTANCNFSM5PH3L2JA>
   > .
   > Triage notifications on the go with GitHub Mobile for iOS
   > <https://apps.apple.com/app/apple-store/id1477376905?ct=notification-email&mt=8&pt=524675>
   > or Android
   > <https://play.google.com/store/apps/details?id=com.github.android&referrer=utm_campaign%3Dnotification-email%26utm_medium%3Demail%26utm_source%3Dgithub>.
   >
   > You are receiving this because you authored the thread.Message ID:
   > ***@***.***>
   >
   


-- 
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] Dev232001 commented on a change in pull request #21796: Replaced all days_ago functions with datetime functions

Posted by GitBox <gi...@apache.org>.
Dev232001 commented on a change in pull request #21796:
URL: https://github.com/apache/airflow/pull/21796#discussion_r815396475



##########
File path: tests/api_connexion/endpoints/test_extra_link_endpoint.py
##########
@@ -95,7 +94,7 @@ def _create_dag():
         with DAG(
             dag_id="TEST_DAG_ID",
             default_args=dict(
-                start_date=days_ago(2),
+                start_date=datetime.datetime(2022, 1, 1),

Review comment:
       That one is also done in the previous pull request. I guess my task is done so please let me know if there is anything more to update. 




-- 
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] josh-fell commented on a change in pull request #21796: Replaced all days_ago functions with datetime functions

Posted by GitBox <gi...@apache.org>.
josh-fell commented on a change in pull request #21796:
URL: https://github.com/apache/airflow/pull/21796#discussion_r814097594



##########
File path: tests/api_connexion/endpoints/test_extra_link_endpoint.py
##########
@@ -95,7 +94,7 @@ def _create_dag():
         with DAG(
             dag_id="TEST_DAG_ID",
             default_args=dict(
-                start_date=days_ago(2),
+                start_date=datetime.datetime(2022, 1, 1),

Review comment:
       I suspect we should move away from `datetime.datetime` to `pendulum.datetime` for `start_date` values in unit tests too based on the conversation in #21646. @eladkal 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] Dev232001 commented on a change in pull request #21796: Replaced all days_ago functions with datetime functions

Posted by GitBox <gi...@apache.org>.
Dev232001 commented on a change in pull request #21796:
URL: https://github.com/apache/airflow/pull/21796#discussion_r815290406



##########
File path: tests/api_connexion/endpoints/test_extra_link_endpoint.py
##########
@@ -95,7 +94,7 @@ def _create_dag():
         with DAG(
             dag_id="TEST_DAG_ID",
             default_args=dict(
-                start_date=days_ago(2),
+                start_date=datetime.datetime(2022, 1, 1),

Review comment:
       https://github.com/apache/airflow/pull/21830/commits/80a6ae5c52a2091774bd4451de4a75ba5097767c
   
   Another pull request after swapping datetime with pendulum where start dates are involved




-- 
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 #21796: Replaced all days_ago functions with datetime functions

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


   @Dev232001 do you need further help?


-- 
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] Dev232001 commented on a change in pull request #21796: Replaced all days_ago functions with datetime functions

Posted by GitBox <gi...@apache.org>.
Dev232001 commented on a change in pull request #21796:
URL: https://github.com/apache/airflow/pull/21796#discussion_r815287679



##########
File path: tests/api_connexion/endpoints/test_extra_link_endpoint.py
##########
@@ -95,7 +94,7 @@ def _create_dag():
         with DAG(
             dag_id="TEST_DAG_ID",
             default_args=dict(
-                start_date=days_ago(2),
+                start_date=datetime.datetime(2022, 1, 1),

Review comment:
       Shall I replace datetime.timedelta() with pendulum.duration() in situations like these -
    default_args={'start_date': test_start_date, 'sla': datetime.timedelta()},




-- 
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] Dev232001 commented on a change in pull request #21796: Replaced all days_ago functions with datetime functions

Posted by GitBox <gi...@apache.org>.
Dev232001 commented on a change in pull request #21796:
URL: https://github.com/apache/airflow/pull/21796#discussion_r814096642



##########
File path: tests/dags/test_example_bash_operator.py
##########
@@ -16,13 +16,12 @@
 # specific language governing permissions and limitations
 # under the License.
 from datetime import timedelta
-
+import datetime

Review comment:
       oh! didn't see that there. There are a lot of files and I checked each one manually for maximum efficiency so forgot to use some of the already present libraries. If you want I can change them again quickly




-- 
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 a change in pull request #21796: Replaced all days_ago functions with datetime functions

Posted by GitBox <gi...@apache.org>.
eladkal commented on a change in pull request #21796:
URL: https://github.com/apache/airflow/pull/21796#discussion_r814063683



##########
File path: tests/dags/test_example_bash_operator.py
##########
@@ -16,13 +16,12 @@
 # specific language governing permissions and limitations
 # under the License.
 from datetime import timedelta
-
+import datetime

Review comment:
       If `from datetime import timedelta` already present why not doing
   `from datetime import datetime, timedelta`
   
   and use `datetime(2022,1,1)` rather than `datetime.datetime(2022,1,1)`




-- 
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] Dev232001 commented on pull request #21796: Replaced all days_ago functions with datetime functions

Posted by GitBox <gi...@apache.org>.
Dev232001 commented on pull request #21796:
URL: https://github.com/apache/airflow/pull/21796#issuecomment-1059247502


   I tried solving the static checks but wasn't able to do so. Can you help me on what is wrong exactly


-- 
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] Dev232001 commented on a change in pull request #21796: Replaced all days_ago functions with datetime functions

Posted by GitBox <gi...@apache.org>.
Dev232001 commented on a change in pull request #21796:
URL: https://github.com/apache/airflow/pull/21796#discussion_r814116781



##########
File path: tests/api_connexion/endpoints/test_extra_link_endpoint.py
##########
@@ -95,7 +94,7 @@ def _create_dag():
         with DAG(
             dag_id="TEST_DAG_ID",
             default_args=dict(
-                start_date=days_ago(2),
+                start_date=datetime.datetime(2022, 1, 1),

Review comment:
       Will it be ok if I do this after Friday. I am currently having semester exams which will be finished by Friday.
   




-- 
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 a change in pull request #21796: Replaced all days_ago functions with datetime functions

Posted by GitBox <gi...@apache.org>.
eladkal commented on a change in pull request #21796:
URL: https://github.com/apache/airflow/pull/21796#discussion_r814102924



##########
File path: tests/api_connexion/endpoints/test_extra_link_endpoint.py
##########
@@ -95,7 +94,7 @@ def _create_dag():
         with DAG(
             dag_id="TEST_DAG_ID",
             default_args=dict(
-                start_date=days_ago(2),
+                start_date=datetime.datetime(2022, 1, 1),

Review comment:
       Do you mean just for tests or also for example dags?
   I have no objection.




-- 
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] Dev232001 commented on a change in pull request #21796: Replaced all days_ago functions with datetime functions

Posted by GitBox <gi...@apache.org>.
Dev232001 commented on a change in pull request #21796:
URL: https://github.com/apache/airflow/pull/21796#discussion_r814108862



##########
File path: tests/api_connexion/endpoints/test_extra_link_endpoint.py
##########
@@ -95,7 +94,7 @@ def _create_dag():
         with DAG(
             dag_id="TEST_DAG_ID",
             default_args=dict(
-                start_date=days_ago(2),
+                start_date=datetime.datetime(2022, 1, 1),

Review comment:
       I too think pendulum is a better alternative for datetime




-- 
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 a change in pull request #21796: Replaced all days_ago functions with datetime functions

Posted by GitBox <gi...@apache.org>.
eladkal commented on a change in pull request #21796:
URL: https://github.com/apache/airflow/pull/21796#discussion_r814114855



##########
File path: tests/api_connexion/endpoints/test_extra_link_endpoint.py
##########
@@ -95,7 +94,7 @@ def _create_dag():
         with DAG(
             dag_id="TEST_DAG_ID",
             default_args=dict(
-                start_date=days_ago(2),
+                start_date=datetime.datetime(2022, 1, 1),

Review comment:
       Then yeah lets do 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] uranusjr commented on a change in pull request #21796: Replaced all days_ago functions with datetime functions

Posted by GitBox <gi...@apache.org>.
uranusjr commented on a change in pull request #21796:
URL: https://github.com/apache/airflow/pull/21796#discussion_r815296676



##########
File path: tests/api_connexion/endpoints/test_extra_link_endpoint.py
##########
@@ -95,7 +94,7 @@ def _create_dag():
         with DAG(
             dag_id="TEST_DAG_ID",
             default_args=dict(
-                start_date=days_ago(2),
+                start_date=datetime.datetime(2022, 1, 1),

Review comment:
       For tests specifically I actually prefer `datetime.datetime` since it better ensures we are coercing timezones correctly internally.




-- 
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] Bowrna commented on pull request #21796: Replaced all days_ago functions with datetime functions

Posted by GitBox <gi...@apache.org>.
Bowrna commented on pull request #21796:
URL: https://github.com/apache/airflow/pull/21796#issuecomment-1059929306


   @Dev232001 run `pre-commit run --all-files`  locally in your machine to fix the black formatting, sorting issues. There are a few duplicate imports too. If you run the command locally it will point to the files with those errors. you can fix it and commit to avoid static check issues.
   
   Alternatively you can run static check via Breeze too. Under the hood this runs pre-commit too. 
   https://github.com/apache/airflow/blob/main/BREEZE.rst#running-static-checks


-- 
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] Dev232001 commented on a change in pull request #21796: Replaced all days_ago functions with datetime functions

Posted by GitBox <gi...@apache.org>.
Dev232001 commented on a change in pull request #21796:
URL: https://github.com/apache/airflow/pull/21796#discussion_r815287679



##########
File path: tests/api_connexion/endpoints/test_extra_link_endpoint.py
##########
@@ -95,7 +94,7 @@ def _create_dag():
         with DAG(
             dag_id="TEST_DAG_ID",
             default_args=dict(
-                start_date=days_ago(2),
+                start_date=datetime.datetime(2022, 1, 1),

Review comment:
       Shall I replace datetime.timedelta() with pendulum.duration() in situations like these -
    default_args={'start_date': test_start_date, 'sla': datetime.timedelta()},




-- 
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] Dev232001 commented on a change in pull request #21796: Replaced all days_ago functions with datetime functions

Posted by GitBox <gi...@apache.org>.
Dev232001 commented on a change in pull request #21796:
URL: https://github.com/apache/airflow/pull/21796#discussion_r814281185



##########
File path: tests/api_connexion/endpoints/test_extra_link_endpoint.py
##########
@@ -95,7 +94,7 @@ def _create_dag():
         with DAG(
             dag_id="TEST_DAG_ID",
             default_args=dict(
-                start_date=days_ago(2),
+                start_date=datetime.datetime(2022, 1, 1),

Review comment:
       ok thanks




-- 
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 #21796: Replaced all days_ago functions with datetime functions

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


   @Dev232001 do you need further help?


-- 
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] boring-cyborg[bot] commented on pull request #21796: Replaced all days_ago functions with datetime functions

Posted by GitBox <gi...@apache.org>.
boring-cyborg[bot] commented on pull request #21796:
URL: https://github.com/apache/airflow/pull/21796#issuecomment-1050031805


   Congratulations on your first Pull Request and welcome to the Apache Airflow community! If you have any issues or are unsure about any anything please check our Contribution Guide (https://github.com/apache/airflow/blob/main/CONTRIBUTING.rst)
   Here are some useful points:
   - Pay attention to the quality of your code (flake8, mypy and type annotations). Our [pre-commits]( https://github.com/apache/airflow/blob/main/STATIC_CODE_CHECKS.rst#prerequisites-for-pre-commit-hooks) will help you with that.
   - In case of a new feature add useful documentation (in docstrings or in `docs/` directory). Adding a new operator? Check this short [guide](https://github.com/apache/airflow/blob/main/docs/apache-airflow/howto/custom-operator.rst) Consider adding an example DAG that shows how users should use it.
   - Consider using [Breeze environment](https://github.com/apache/airflow/blob/main/BREEZE.rst) for testing locally, it’s a heavy docker but it ships with a working Airflow and a lot of integrations.
   - Be patient and persistent. It might take some time to get a review or get the final approval from Committers.
   - Please follow [ASF Code of Conduct](https://www.apache.org/foundation/policies/conduct) for all communication including (but not limited to) comments on Pull Requests, Mailing list and Slack.
   - Be sure to read the [Airflow Coding style]( https://github.com/apache/airflow/blob/main/CONTRIBUTING.rst#coding-style-and-best-practices).
   Apache Airflow is a community-driven project and together we are making it better πŸš€.
   In case of doubts contact the developers at:
   Mailing List: dev@airflow.apache.org
   Slack: https://s.apache.org/airflow-slack
   


-- 
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] josh-fell commented on a change in pull request #21796: Replaced all days_ago functions with datetime functions

Posted by GitBox <gi...@apache.org>.
josh-fell commented on a change in pull request #21796:
URL: https://github.com/apache/airflow/pull/21796#discussion_r814280058



##########
File path: tests/api_connexion/endpoints/test_extra_link_endpoint.py
##########
@@ -95,7 +94,7 @@ def _create_dag():
         with DAG(
             dag_id="TEST_DAG_ID",
             default_args=dict(
-                start_date=days_ago(2),
+                start_date=datetime.datetime(2022, 1, 1),

Review comment:
       Of course! Not a problem at all




-- 
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 #21796: Replaced all days_ago functions with datetime functions

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


   Please fix the static check failures.


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