jscheffl merged PR #35096:
URL: https://github.com/apache/airflow/pull/35096
--
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-unsubscr...@airflow
boring-cyborg[bot] commented on PR #35096:
URL: https://github.com/apache/airflow/pull/35096#issuecomment-1811249137
Awesome work, congrats on your first merged pull request! You are invited to
check our [Issue Tracker](https://github.com/apache/airflow/issues) for
additional contributions.
PashkPashk commented on PR #35096:
URL: https://github.com/apache/airflow/pull/35096#issuecomment-1811130179
@jscheffl
Oops, I thought static checks has been applied automatically.
Fixed, I hope will work this time
--
This is an automated message from the Apache Git Service.
To resp
jscheffl commented on PR #35096:
URL: https://github.com/apache/airflow/pull/35096#issuecomment-1806932769
Still no luck with static checks, can you take a look here? Actually it just
seems you need to run it once and commit the changed applied from ruff:
https://github.com/apache/airflow/b
jscheffl commented on PR #35096:
URL: https://github.com/apache/airflow/pull/35096#issuecomment-1802855199
Just small glitches in static code checks. If you remove them I assume it is
good to get it merged.
--
This is an automated message from the Apache Git Service.
To respond to the mes
PashkPashk commented on code in PR #35096:
URL: https://github.com/apache/airflow/pull/35096#discussion_r1386758077
##
airflow/models/dagrun.py:
##
@@ -258,6 +281,14 @@ def validate_run_id(self, key: str, run_id: str) -> str |
None:
)
return run_id
+
PashkPashk commented on PR #35096:
URL: https://github.com/apache/airflow/pull/35096#issuecomment-1802057847
I’ve been mistaken about hybrid_property, cause it doesn’t work correctly
with mypy, should be using declared_attr instead.
I also am not really sure how to deal with test_seriali
uranusjr commented on code in PR #35096:
URL: https://github.com/apache/airflow/pull/35096#discussion_r1384407836
##
airflow/models/dagrun.py:
##
@@ -258,6 +281,14 @@ def validate_run_id(self, key: str, run_id: str) -> str |
None:
)
return run_id
+@h
jens-scheffler-bosch commented on PR #35096:
URL: https://github.com/apache/airflow/pull/35096#issuecomment-1785293149
There are some (simple) static code check problems. Before making an
approval review I'd like see them fixed. But "okay" from my side. I am not an
expert in sqlachemy so I
jens-scheffler-bosch commented on PR #35096:
URL: https://github.com/apache/airflow/pull/35096#issuecomment-1781676216
> > This looks good. Thanks for the contribution.
> > I was thinging (but have no strong opinion) as whether it makes sense to
extract this to a utility, but keeping it h
PashkPashk commented on PR #35096:
URL: https://github.com/apache/airflow/pull/35096#issuecomment-1778140569
> This looks good. Thanks for the contribution.
>
> I was thinging (but have no strong opinion) as whether it makes sense to
extract this to a utility, but keeping it here migh
PashkPashk commented on code in PR #35096:
URL: https://github.com/apache/airflow/pull/35096#discussion_r1370693317
##
airflow/models/dagrun.py:
##
@@ -126,7 +150,7 @@ class DagRun(Base, LoggingMixin):
creating_job_id = Column(Integer)
external_trigger = Column(Boolean
PashkPashk commented on code in PR #35096:
URL: https://github.com/apache/airflow/pull/35096#discussion_r1370692991
##
airflow/models/dagrun.py:
##
@@ -210,7 +234,7 @@ def __init__(
execution_date: datetime | None = None,
start_date: datetime | None = None,
jens-scheffler-bosch commented on code in PR #35096:
URL: https://github.com/apache/airflow/pull/35096#discussion_r1369048257
##
airflow/models/dagrun.py:
##
@@ -126,7 +150,7 @@ class DagRun(Base, LoggingMixin):
creating_job_id = Column(Integer)
external_trigger = Colu
uranusjr commented on code in PR #35096:
URL: https://github.com/apache/airflow/pull/35096#discussion_r1368358997
##
airflow/models/dagrun.py:
##
@@ -126,7 +150,7 @@ class DagRun(Base, LoggingMixin):
creating_job_id = Column(Integer)
external_trigger = Column(Boolean,
uranusjr commented on code in PR #35096:
URL: https://github.com/apache/airflow/pull/35096#discussion_r1368356565
##
airflow/models/dagrun.py:
##
@@ -210,7 +234,7 @@ def __init__(
execution_date: datetime | None = None,
start_date: datetime | None = None,
uranusjr commented on code in PR #35096:
URL: https://github.com/apache/airflow/pull/35096#discussion_r1368356030
##
airflow/models/dagrun.py:
##
@@ -228,7 +252,7 @@ def __init__(
self.execution_date = execution_date
self.start_date = start_date
self.e
eladkal commented on PR #35096:
URL: https://github.com/apache/airflow/pull/35096#issuecomment-1773721000
> Can you please add a unit test for the new code which validates a positive
and negative case as well?
yes unit test is required here
--
This is an automated message from the
boring-cyborg[bot] commented on PR #35096:
URL: https://github.com/apache/airflow/pull/35096#issuecomment-1773530032
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
PashkPashk opened a new pull request, #35096:
URL: https://github.com/apache/airflow/pull/35096
Closes #35095
--
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 unsubscrib
20 matches
Mail list logo
