potiuk commented on pull request #17576:
URL: https://github.com/apache/airflow/pull/17576#issuecomment-898028425
> Is the goal of this PR to replace & generalize this use case or the real
issue you are trying to solve is something else ?
Something completely different. Pre/postopera
potiuk commented on pull request #17576:
URL: https://github.com/apache/airflow/pull/17576#issuecomment-898029293
LGTM - but serialisation needs to be updated to skip those new fields (tests
are failing).
--
This is an automated message from the Apache Git Service.
To respond to the mess
potiuk commented on pull request #17576:
URL: https://github.com/apache/airflow/pull/17576#issuecomment-898261046
Looking at the test, there must be a reason the test it here (not only to
annoy the user) and it says explicitly that new fields should not be added, so
that makes me wonder if
potiuk commented on pull request #17576:
URL: https://github.com/apache/airflow/pull/17576#issuecomment-901396372
Hey @kaxil -> do you think we can just add _pre/_post to the test?
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to Git
potiuk commented on pull request #17576:
URL: https://github.com/apache/airflow/pull/17576#issuecomment-903326972
Hey @kaxil - any comments here? And @malthe - woudl you please rebase after
the comment from @kaxil?
--
This is an automated message from the Apache Git Service.
To respond t
potiuk commented on pull request #17576:
URL: https://github.com/apache/airflow/pull/17576#issuecomment-903329602
I was asking @kaxil for comment about the serialisation and reason why the
tests explicitly mention come fields :)
--
This is an automated message from the Apache Git Servic
potiuk commented on pull request #17576:
URL: https://github.com/apache/airflow/pull/17576#issuecomment-904010653
> These two new fields you add don't need to be serialized (and callables
can't generally be anyway) -- the general rule is that things needed by the
Scheduler should be serial
potiuk commented on pull request #17576:
URL: https://github.com/apache/airflow/pull/17576#issuecomment-904457121
Hey @malthe - I think this one, will be very powerful. After some
discussions on Slack recently, the potential of those overrideable hooks is
very interesting for a number of
potiuk commented on pull request #17576:
URL: https://github.com/apache/airflow/pull/17576#issuecomment-916379253
One of the good cases is to distinguish between DEV/PROD environments.
For example some of the operators could be skipped on DEV environments and
you might want to add
potiuk commented on pull request #17576:
URL: https://github.com/apache/airflow/pull/17576#issuecomment-916395891
Just to add one more thing @ashb - if there are other features in upcoming
AIP's that might make it useless/obsolete/outdated/replaceable - I am all for
it. We just simply migh
potiuk commented on pull request #17576:
URL: https://github.com/apache/airflow/pull/17576#issuecomment-916426991
I think I even discussed it with @malthe that time based decision will be
better done based on AIP-39. So I agree @ashb - this is likely not a good use
of those pre/post_hooks
potiuk commented on pull request #17576:
URL: https://github.com/apache/airflow/pull/17576#issuecomment-916792442
> I'd like to get 2.2 release train started, and in order to not delay that
unnecessarily how about we mark this feature as experimental -- that way if we
decide we _don't_ wan
potiuk commented on pull request #17576:
URL: https://github.com/apache/airflow/pull/17576#issuecomment-916793392
We can even print warning when it is used, that it is experimental.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to Gi
potiuk commented on pull request #17576:
URL: https://github.com/apache/airflow/pull/17576#issuecomment-920781011
I think that might be indeed why @ashb insisted on making that feature
experimental - because I see that each one of us has different "use case" in
mind when looking at the pre
potiuk commented on pull request #17576:
URL: https://github.com/apache/airflow/pull/17576#issuecomment-986363233
Another possibly interesting use case for pre_execute:
https://apache-airflow.slack.com/archives/CCVDRN0F9/p1638740065139100
Basically adding a "session" attri
15 matches
Mail list logo
