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 2020/12/08 17:44:53 UTC

[GitHub] [airflow] dstandish edited a comment on pull request #12910: Fix return type in prev-date context variables

dstandish edited a comment on pull request #12910:
URL: https://github.com/apache/airflow/pull/12910#issuecomment-740792150


   since pendulum inherits from datetime, i think the only thing that really breaks is string representation, though i could be wrong.
   
   apologies for providing us with a conundrum here.  it was one of my first contributions if not the first.  
   
   i took a look and i remember what was going on.  at the time, prev_execution_date and next_execution_date returned native datetime despite docs saying pendulum and execution_date being pendulum.  So when I added prev_execution_date_success etc, I kept the behavior and documentation consistent with that of prev_execution_date.  Why I didn't just wrap it in pendulum.instance, no clue.  Maybe I thought that under the hood execution_date was supposed to be always pendulum and this was merely a bug that would be fixed.
   
   In PR https://github.com/apache/airflow/pull/5654, this was partially fixed.  But the _success attributes were not updated.  So while now prev_execution_date returns pendulum, the _success versions do not.
   
   If this can be fixed for prev_execution_date, as it was in https://github.com/apache/airflow/pull/5654, I suppose it can be fixed for prev_execution_date_success.
   
   


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

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