You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@airflow.apache.org by "uranusjr (via GitHub)" <gi...@apache.org> on 2024/11/22 08:49:46 UTC
[PR] Add logical_date to models using execution_date [airflow]
uranusjr opened a new pull request, #44283:
URL: https://github.com/apache/airflow/pull/44283
Airflow 3 renames all database columns from 'execution_date' to 'logical_date'. This backports the new name to 2.x so users can change their code accessing those fields to use the new name before upgrading to Airflow 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
Re: [PR] Add logical_date to models using execution_date [airflow]
Posted by "uranusjr (via GitHub)" <gi...@apache.org>.
uranusjr merged PR #44283:
URL: https://github.com/apache/airflow/pull/44283
--
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
Re: [PR] Add logical_date to models using execution_date [airflow]
Posted by "ephraimbuddy (via GitHub)" <gi...@apache.org>.
ephraimbuddy commented on code in PR #44283:
URL: https://github.com/apache/airflow/pull/44283#discussion_r1855890653
##########
airflow/models/slamiss.py:
##########
@@ -44,3 +49,7 @@ class SlaMiss(Base):
def __repr__(self):
return str((self.dag_id, self.task_id, self.execution_date.isoformat()))
+
+ @property
Review Comment:
> Likely keeping such changes as DRAFT PR and rebasing them individually/merging PRs later is a better idea - as it puts the burden on solving the issue on the one who is author (i.e. knows best what the intention and context was).
I prefer this
--
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
Re: [PR] Add logical_date to models using execution_date [airflow]
Posted by "potiuk (via GitHub)" <gi...@apache.org>.
potiuk commented on code in PR #44283:
URL: https://github.com/apache/airflow/pull/44283#discussion_r1855522653
##########
airflow/models/slamiss.py:
##########
@@ -44,3 +49,7 @@ class SlaMiss(Base):
def __repr__(self):
return str((self.dag_id, self.task_id, self.execution_date.isoformat()))
+
+ @property
Review Comment:
We could also potentially have `v2-11-test` branch at some point of time (this is mostly the matter of how many changes like that we are going to have). Theorethically, there is nothing wrong with having both branches created and having Pull Requests to v2-11-test. We could then later rebase v2-11-test on top of v2-10-test. - but that basically means that someone who merges changes now would have to resolve the conflicts if that happens - or maybe put the whole weitght of this resolving on the shoulders of release manager - both are not too good ideas, likely keeping such changes as DRAFT PR and rebasing them individually/merging PRs later is a better idea - as it puts the burden on solving the issue on the one who is author (i.e. knows best what the intention and context was).
--
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
Re: [PR] Add logical_date to models using execution_date [airflow]
Posted by "ephraimbuddy (via GitHub)" <gi...@apache.org>.
ephraimbuddy commented on code in PR #44283:
URL: https://github.com/apache/airflow/pull/44283#discussion_r1853634219
##########
airflow/models/slamiss.py:
##########
@@ -44,3 +49,7 @@ class SlaMiss(Base):
def __repr__(self):
return str((self.dag_id, self.task_id, self.execution_date.isoformat()))
Review Comment:
Should the above self.execution_date be changed too?
--
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
Re: [PR] Add logical_date to models using execution_date [airflow]
Posted by "Lee-W (via GitHub)" <gi...@apache.org>.
Lee-W commented on PR #44283:
URL: https://github.com/apache/airflow/pull/44283#issuecomment-2662402958
@utkarsharma2 will we still have 2-10 releases? If not, should we mark it as ready to review and get it merged
--
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
Re: [PR] Add logical_date to models using execution_date [airflow]
Posted by "uranusjr (via GitHub)" <gi...@apache.org>.
uranusjr commented on code in PR #44283:
URL: https://github.com/apache/airflow/pull/44283#discussion_r1853735006
##########
airflow/models/slamiss.py:
##########
@@ -44,3 +49,7 @@ class SlaMiss(Base):
def __repr__(self):
return str((self.dag_id, self.task_id, self.execution_date.isoformat()))
Review Comment:
Doesn’t matter since the two point to the same thing. 2.10 has a lot of things using `execution_date`, let’s not bother changing this.
--
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
Re: [PR] Add logical_date to models using execution_date [airflow]
Posted by "potiuk (via GitHub)" <gi...@apache.org>.
potiuk commented on PR #44283:
URL: https://github.com/apache/airflow/pull/44283#issuecomment-2663044437
> @utkarsharma2 will we still have 2-10 releases? If not, should we mark it as ready to review and get it merged
Good question :)
--
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
Re: [PR] Add logical_date to models using execution_date [airflow]
Posted by "ephraimbuddy (via GitHub)" <gi...@apache.org>.
ephraimbuddy commented on code in PR #44283:
URL: https://github.com/apache/airflow/pull/44283#discussion_r1855395802
##########
airflow/models/slamiss.py:
##########
@@ -44,3 +49,7 @@ class SlaMiss(Base):
def __repr__(self):
return str((self.dag_id, self.task_id, self.execution_date.isoformat()))
+
+ @property
Review Comment:
I think we should block merging it to v2-10-test until we stop 2.10.* releases
--
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
Re: [PR] Add logical_date to models using execution_date [airflow]
Posted by "potiuk (via GitHub)" <gi...@apache.org>.
potiuk commented on code in PR #44283:
URL: https://github.com/apache/airflow/pull/44283#discussion_r1855522653
##########
airflow/models/slamiss.py:
##########
@@ -44,3 +49,7 @@ class SlaMiss(Base):
def __repr__(self):
return str((self.dag_id, self.task_id, self.execution_date.isoformat()))
+
+ @property
Review Comment:
We could also potentially have `v2-11-test` branch at some point of time (this is mostly the matter of how many changes like that we are going to have). Theorethically, there is nothing wrong with having both branches created and having Pull Requests to v2-11-test. We could then later rebase v2-11-test on top of v2-10-test. - but that basically means that someone who merges changes now would have to resolve the conflicts if that happens - or maybe put the whole weitght of this resolving on the shoulders of release manager - both are not too good ideas.
Likely keeping such changes as DRAFT PR and rebasing them individually/merging PRs later is a better idea - as it puts the burden on solving the issue on the one who is author (i.e. knows best what the intention and context was).
--
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