airflow-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From Jarek Potiuk <Jarek.Pot...@polidea.com>
Subject Re: AIP-22: Group ORM models by their logical usage instead of type
Date Sun, 23 Jun 2019 23:07:49 GMT
Just few comments to spark a discussion about backwards compatibility for
this proposal:

-  Airflow uses Alembic to generate automated migration scripts. Not sure
where it takes all the models from to generate the scripts (__init__ in
airflow.models maybe, or simply by scanning all classes). I do not expect
any serious problems as alembic uses column/table names in its migration
scripts but it would be worth checking if there are any negative
consequences
- I think it might make sense to keep airflow.models.__init__ in place
anyway to keep backwards compatibility maybe ?
- I think there is one particular case which is super-sensitive for
backwards compatibility models.DAG - which is used pretty much in all DAGs
written by anyone (`from airflow.models import DAG` or `with
models.DAG(...` ). I think it's one thing to break some backwards
compatibility in edge cases and it's quite a different thing if pretty much
all custom-written DAGs have to be corrected. I think this is yet another
argument to keep at least the old __init__ and import the classes from
their new locations still keeping the old 'models' package - even if we
decide to move them. And then it might turn out the change will be fully
backwards-compatible.

J.

On Sat, Jun 22, 2019 at 8:50 PM Bas Harenslak <basharenslak@godatadriven.com>
wrote:

> Hi all,
>
> I’ve created AIP-22 and propose to move every ORM class to a module where
> one would logically expect them to be, instead of grouping all ORM models
> together, e.g. BaseOperator in airflow.operators (this is actually not an
> ORM class).
>
> To me the benefit would be a more logically structured code base, with
> classes stored where you’d initially expect them to be, instead of having
> to know upfront if they’re ORM classes or not.
>
> I’ve explained the plan in more detail in the AIP:
> https://cwiki.apache.org/confluence/display/AIRFLOW/AIP-22%3A+Group+ORM+models+by+their+logical+usage+instead+of+type
> <
> https://cwiki.apache.org/confluence/display/AIRFLOW/AIP-22:+Group+ORM+models+by+their+logical+usage+instead+of+type>
> and wonder if others think the same about it.
>
> Needless to say, this would be a breaking change and only possible in
> Airflow 2.0.
>
> Cheers,
> Bas
>


-- 

Jarek Potiuk
Polidea <https://www.polidea.com/> | Principal Software Engineer

M: +48 660 796 129 <+48660796129>
[image: Polidea] <https://www.polidea.com/>

Mime
  • Unnamed multipart/alternative (inline, None, 0 bytes)
View raw message