Re: Refactor models.py
Andreas good idea, without that the only way to refactor models.py is a big bang all at once. I made a start and renamed models.py to /airflow/models/__init__.py and moved class Connection to /airflow/models/connection.py (AIRFLOW-3458). PR is here: https://github.com/apache/incubator-airflow/pull/4335. Still waiting for all tests to complete. If this works okay, I could continue and split off other classes. Once the last PR of Fokko’s list is merged we should ensure /airflow/models/__init__.py is empty. Also like Tao Feng says, it’s indeed wise to close as many PRs as possible first. On 17 Dec 2018, at 19:59, Tao Feng mailto:fengta...@gmail.com>> wrote: We should merge the existing prs before doing this refactors. Otherwise, there will be so many rebase issues. On Mon, Dec 17, 2018 at 12:37 AM Andreas Költringer < andreas.koeltrin...@n-fuse.co<mailto:andreas.koeltrin...@n-fuse.co>> wrote: Hi, a suggestion to make this process easier: a folder could be created called `models`. The `models.py` could then moved into that folder and renamed to `__init__.py`. That way, all the other parts of airflow can be left untouched - it is still possible to run `from models import ` In the next steps, classes can be moved out of the now called `__init__.py` into separate files. The 'moved' class then needs to be imported in `__init__.py` - to not affect the rest of airflow. Example: move class `User` to `models/user.py`. In `models/__init__.py` add `from .user import User`. What do you think? On Thursday, December 6, 2018 1:08:34 PM CET Ash Berlin-Taylor wrote: Hi Fokko, I commented on some of the issues -we could possibly just delete User and KnownEvent* My suggestion is to create a new package, called models, which will contain all the orm classes And do what with the current airflow.models? Do you have an idea of module names to move things to? Are you thinking we have airflow.models.connection module containing just a Connection class for example? -ash On 6 Dec 2018, at 11:35, Driesprong, Fokko mailto:fo...@driesprong.frl>> wrote: Hi All, I think it is time to refactor the infamous models.py. This file is far too big, and it only keeps growing. My suggestion is to create a new package, called models, which will contain all the orm classes (the ones with __tablename__ in the class). And for example the BaseOperator to the operator packages. I've created a lot of tickets to move the classes one by one out of models.py. The reason to do this one by one is to relieve the pain of fixing the circular dependencies. Refactor: Move DagBag out of models.py https://issues.apache.org/jira/browse/AIRFLOW-3456 Refactor: Move User out of models.py https://issues.apache.org/jira/browse/AIRFLOW-3457 Refactor: Move Connection out of models.py https://issues.apache.org/jira/browse/AIRFLOW-3458 Refactor: Move DagPickle out of models.py https://issues.apache.org/jira/browse/AIRFLOW-3459 Refactor: Move TaskInstance out of models.py https://issues.apache.org/jira/browse/AIRFLOW-3460 Refactor: Move TaskFail out of models.py https://issues.apache.org/jira/browse/AIRFLOW-3461 Refactor: Move TaskReschedule out of models.py https://issues.apache.org/jira/browse/AIRFLOW-3462 Refactor: Move Log out of models.py https://issues.apache.org/jira/browse/AIRFLOW-3463 Refactor: Move SkipMixin out of models.py https://issues.apache.org/jira/browse/AIRFLOW-3464 Refactor: Move BaseOperator out of models.py https://issues.apache.org/jira/browse/AIRFLOW-3465 Refactor: Move DAG out of models.py https://issues.apache.org/jira/browse/AIRFLOW-3466 Refactor: Move Chart out of models.py https://issues.apache.org/jira/browse/AIRFLOW-3467 Refactor: Move KnownEventType out of models.py https://issues.apache.org/jira/browse/AIRFLOW-3468 Refactor: Move KnownEvent out of models.py https://issues.apache.org/jira/browse/AIRFLOW-3469 Refactor: Move Variable out of models.py https://issues.apache.org/jira/browse/AIRFLOW-3470 Refactor: Move XCom out of models.py https://issues.apache.org/jira/browse/AIRFLOW-3471 Refactor: Move DagStat out of models.py https://issues.apache.org/jira/browse/AIRFLOW-3472 Refactor: Move DagRun out of models.py https://issues.apache.org/jira/browse/AIRFLOW-3473 Refactor: Move SlaMiss out of models.py https://issues.apache.org/jira/browse/AIRFLOW-3474 Refactor: Move ImportError out of models.py https://issues.apache.org/jira/browse/AIRFLOW-3475 Refactor: Move KubeResourceVersion out of models.py https://issues.apache.org/jira/browse/AIRFLOW-3476 Refactor: Move KubeWorkerIdentifier out of models.py https://issues.apache.org/jira/browse/AIRFLOW-3477 Some classes are really simple, and would also be a nice opportunity for newcomers to start contributing to Airflow :-) Cheers, Fokko -- Andreas Koeltringer Mail: andreas.koeltrin...@n-fuse.co<mailto:andreas.koeltrin...@n-fuse.co>
Re: Refactor models.py
We should merge the existing prs before doing this refactors. Otherwise, there will be so many rebase issues. On Mon, Dec 17, 2018 at 12:37 AM Andreas Költringer < andreas.koeltrin...@n-fuse.co> wrote: > Hi, > > a suggestion to make this process easier: > > a folder could be created called `models`. The `models.py` could then > moved > into that folder and renamed to `__init__.py`. That way, all the other > parts > of airflow can be left untouched - it is still possible to run > > `from models import ` > > In the next steps, classes can be moved out of the now called > `__init__.py` > into separate files. The 'moved' class then needs to be imported in > `__init__.py` - to not affect the rest of airflow. > > Example: move class `User` to `models/user.py`. In `models/__init__.py` > add > `from .user import User`. > > What do you think? > > > On Thursday, December 6, 2018 1:08:34 PM CET Ash Berlin-Taylor wrote: > > Hi Fokko, > > > > I commented on some of the issues -we could possibly just delete User and > > KnownEvent* > > > My suggestion is to create a new package, called models, which will > > > contain all the orm classes > > And do what with the current airflow.models? > > > > Do you have an idea of module names to move things to? Are you thinking > we > > have airflow.models.connection module containing just a Connection class > > for example? > > > > -ash > > > > > On 6 Dec 2018, at 11:35, Driesprong, Fokko > wrote: > > > > > > Hi All, > > > > > > I think it is time to refactor the infamous models.py. This file is far > > > too > > > big, and it only keeps growing. My suggestion is to create a new > package, > > > called models, which will contain all the orm classes (the ones > > > with __tablename__ in the class). And for example the BaseOperator to > the > > > operator packages. I've created a lot of tickets to move the classes > one > > > by > > > one out of models.py. The reason to do this one by one is to relieve > the > > > pain of fixing the circular dependencies. > > > > > > Refactor: Move DagBag out of models.py > > > https://issues.apache.org/jira/browse/AIRFLOW-3456 > > > > > > Refactor: Move User out of models.py > > > https://issues.apache.org/jira/browse/AIRFLOW-3457 > > > > > > Refactor: Move Connection out of models.py > > > https://issues.apache.org/jira/browse/AIRFLOW-3458 > > > > > > Refactor: Move DagPickle out of models.py > > > https://issues.apache.org/jira/browse/AIRFLOW-3459 > > > > > > Refactor: Move TaskInstance out of models.py > > > https://issues.apache.org/jira/browse/AIRFLOW-3460 > > > > > > Refactor: Move TaskFail out of models.py > > > https://issues.apache.org/jira/browse/AIRFLOW-3461 > > > > > > Refactor: Move TaskReschedule out of models.py > > > https://issues.apache.org/jira/browse/AIRFLOW-3462 > > > > > > Refactor: Move Log out of models.py > > > https://issues.apache.org/jira/browse/AIRFLOW-3463 > > > > > > Refactor: Move SkipMixin out of models.py > > > https://issues.apache.org/jira/browse/AIRFLOW-3464 > > > > > > Refactor: Move BaseOperator out of models.py > > > https://issues.apache.org/jira/browse/AIRFLOW-3465 > > > > > > Refactor: Move DAG out of models.py > > > https://issues.apache.org/jira/browse/AIRFLOW-3466 > > > > > > Refactor: Move Chart out of models.py > > > https://issues.apache.org/jira/browse/AIRFLOW-3467 > > > > > > Refactor: Move KnownEventType out of models.py > > > https://issues.apache.org/jira/browse/AIRFLOW-3468 > > > > > > Refactor: Move KnownEvent out of models.py > > > https://issues.apache.org/jira/browse/AIRFLOW-3469 > > > > > > Refactor: Move Variable out of models.py > > > https://issues.apache.org/jira/browse/AIRFLOW-3470 > > > > > > Refactor: Move XCom out of models.py > > > https://issues.apache.org/jira/browse/AIRFLOW-3471 > > > > > > Refactor: Move DagStat out of models.py > > > https://issues.apache.org/jira/browse/AIRFLOW-3472 > > > > > > Refactor: Move DagRun out of models.py > > > https://issues.apache.org/jira/browse/AIRFLOW-3473 > > > > > > Refactor: Move SlaMiss out of models.py > > > https://issues.apache.org/jira/browse/AIRFLOW-3474 > > > > > > Refactor: Move ImportError out of models.py > > > https://issues.apache.org/jira/browse/AIRFLOW-3475 > > > > > > Refactor: Move KubeResourceVersion out of models.py > > > https://issues.apache.org/jira/browse/AIRFLOW-3476 > > > > > > Refactor: Move KubeWorkerIdentifier out of models.py > > > https://issues.apache.org/jira/browse/AIRFLOW-3477 > > > > > > Some classes are really simple, and would also be a nice opportunity > for > > > newcomers to start contributing to Airflow :-) > > > > > > Cheers, Fokko > > > -- > Andreas Koeltringer > Mail: andreas.koeltrin...@n-fuse.co > > > > >
Re: Refactor models.py
Hi, a suggestion to make this process easier: a folder could be created called `models`. The `models.py` could then moved into that folder and renamed to `__init__.py`. That way, all the other parts of airflow can be left untouched - it is still possible to run `from models import ` In the next steps, classes can be moved out of the now called `__init__.py` into separate files. The 'moved' class then needs to be imported in `__init__.py` - to not affect the rest of airflow. Example: move class `User` to `models/user.py`. In `models/__init__.py` add `from .user import User`. What do you think? On Thursday, December 6, 2018 1:08:34 PM CET Ash Berlin-Taylor wrote: > Hi Fokko, > > I commented on some of the issues -we could possibly just delete User and > KnownEvent* > > My suggestion is to create a new package, called models, which will > > contain all the orm classes > And do what with the current airflow.models? > > Do you have an idea of module names to move things to? Are you thinking we > have airflow.models.connection module containing just a Connection class > for example? > > -ash > > > On 6 Dec 2018, at 11:35, Driesprong, Fokko wrote: > > > > Hi All, > > > > I think it is time to refactor the infamous models.py. This file is far > > too > > big, and it only keeps growing. My suggestion is to create a new package, > > called models, which will contain all the orm classes (the ones > > with __tablename__ in the class). And for example the BaseOperator to the > > operator packages. I've created a lot of tickets to move the classes one > > by > > one out of models.py. The reason to do this one by one is to relieve the > > pain of fixing the circular dependencies. > > > > Refactor: Move DagBag out of models.py > > https://issues.apache.org/jira/browse/AIRFLOW-3456 > > > > Refactor: Move User out of models.py > > https://issues.apache.org/jira/browse/AIRFLOW-3457 > > > > Refactor: Move Connection out of models.py > > https://issues.apache.org/jira/browse/AIRFLOW-3458 > > > > Refactor: Move DagPickle out of models.py > > https://issues.apache.org/jira/browse/AIRFLOW-3459 > > > > Refactor: Move TaskInstance out of models.py > > https://issues.apache.org/jira/browse/AIRFLOW-3460 > > > > Refactor: Move TaskFail out of models.py > > https://issues.apache.org/jira/browse/AIRFLOW-3461 > > > > Refactor: Move TaskReschedule out of models.py > > https://issues.apache.org/jira/browse/AIRFLOW-3462 > > > > Refactor: Move Log out of models.py > > https://issues.apache.org/jira/browse/AIRFLOW-3463 > > > > Refactor: Move SkipMixin out of models.py > > https://issues.apache.org/jira/browse/AIRFLOW-3464 > > > > Refactor: Move BaseOperator out of models.py > > https://issues.apache.org/jira/browse/AIRFLOW-3465 > > > > Refactor: Move DAG out of models.py > > https://issues.apache.org/jira/browse/AIRFLOW-3466 > > > > Refactor: Move Chart out of models.py > > https://issues.apache.org/jira/browse/AIRFLOW-3467 > > > > Refactor: Move KnownEventType out of models.py > > https://issues.apache.org/jira/browse/AIRFLOW-3468 > > > > Refactor: Move KnownEvent out of models.py > > https://issues.apache.org/jira/browse/AIRFLOW-3469 > > > > Refactor: Move Variable out of models.py > > https://issues.apache.org/jira/browse/AIRFLOW-3470 > > > > Refactor: Move XCom out of models.py > > https://issues.apache.org/jira/browse/AIRFLOW-3471 > > > > Refactor: Move DagStat out of models.py > > https://issues.apache.org/jira/browse/AIRFLOW-3472 > > > > Refactor: Move DagRun out of models.py > > https://issues.apache.org/jira/browse/AIRFLOW-3473 > > > > Refactor: Move SlaMiss out of models.py > > https://issues.apache.org/jira/browse/AIRFLOW-3474 > > > > Refactor: Move ImportError out of models.py > > https://issues.apache.org/jira/browse/AIRFLOW-3475 > > > > Refactor: Move KubeResourceVersion out of models.py > > https://issues.apache.org/jira/browse/AIRFLOW-3476 > > > > Refactor: Move KubeWorkerIdentifier out of models.py > > https://issues.apache.org/jira/browse/AIRFLOW-3477 > > > > Some classes are really simple, and would also be a nice opportunity for > > newcomers to start contributing to Airflow :-) > > > > Cheers, Fokko -- Andreas Koeltringer Mail: andreas.koeltrin...@n-fuse.co
Re: Refactor models.py
Hi Fokko, I commented on some of the issues -we could possibly just delete User and KnownEvent* > My suggestion is to create a new package, called models, which will contain > all the orm classes And do what with the current airflow.models? Do you have an idea of module names to move things to? Are you thinking we have airflow.models.connection module containing just a Connection class for example? -ash > On 6 Dec 2018, at 11:35, Driesprong, Fokko wrote: > > Hi All, > > I think it is time to refactor the infamous models.py. This file is far too > big, and it only keeps growing. My suggestion is to create a new package, > called models, which will contain all the orm classes (the ones > with __tablename__ in the class). And for example the BaseOperator to the > operator packages. I've created a lot of tickets to move the classes one by > one out of models.py. The reason to do this one by one is to relieve the > pain of fixing the circular dependencies. > > Refactor: Move DagBag out of models.py > https://issues.apache.org/jira/browse/AIRFLOW-3456 > > Refactor: Move User out of models.py > https://issues.apache.org/jira/browse/AIRFLOW-3457 > > Refactor: Move Connection out of models.py > https://issues.apache.org/jira/browse/AIRFLOW-3458 > > Refactor: Move DagPickle out of models.py > https://issues.apache.org/jira/browse/AIRFLOW-3459 > > Refactor: Move TaskInstance out of models.py > https://issues.apache.org/jira/browse/AIRFLOW-3460 > > Refactor: Move TaskFail out of models.py > https://issues.apache.org/jira/browse/AIRFLOW-3461 > > Refactor: Move TaskReschedule out of models.py > https://issues.apache.org/jira/browse/AIRFLOW-3462 > > Refactor: Move Log out of models.py > https://issues.apache.org/jira/browse/AIRFLOW-3463 > > Refactor: Move SkipMixin out of models.py > https://issues.apache.org/jira/browse/AIRFLOW-3464 > > Refactor: Move BaseOperator out of models.py > https://issues.apache.org/jira/browse/AIRFLOW-3465 > > Refactor: Move DAG out of models.py > https://issues.apache.org/jira/browse/AIRFLOW-3466 > > Refactor: Move Chart out of models.py > https://issues.apache.org/jira/browse/AIRFLOW-3467 > > Refactor: Move KnownEventType out of models.py > https://issues.apache.org/jira/browse/AIRFLOW-3468 > > Refactor: Move KnownEvent out of models.py > https://issues.apache.org/jira/browse/AIRFLOW-3469 > > Refactor: Move Variable out of models.py > https://issues.apache.org/jira/browse/AIRFLOW-3470 > > Refactor: Move XCom out of models.py > https://issues.apache.org/jira/browse/AIRFLOW-3471 > > Refactor: Move DagStat out of models.py > https://issues.apache.org/jira/browse/AIRFLOW-3472 > > Refactor: Move DagRun out of models.py > https://issues.apache.org/jira/browse/AIRFLOW-3473 > > Refactor: Move SlaMiss out of models.py > https://issues.apache.org/jira/browse/AIRFLOW-3474 > > Refactor: Move ImportError out of models.py > https://issues.apache.org/jira/browse/AIRFLOW-3475 > > Refactor: Move KubeResourceVersion out of models.py > https://issues.apache.org/jira/browse/AIRFLOW-3476 > > Refactor: Move KubeWorkerIdentifier out of models.py > https://issues.apache.org/jira/browse/AIRFLOW-3477 > > Some classes are really simple, and would also be a nice opportunity for > newcomers to start contributing to Airflow :-) > > Cheers, Fokko
Refactor models.py
Hi All, I think it is time to refactor the infamous models.py. This file is far too big, and it only keeps growing. My suggestion is to create a new package, called models, which will contain all the orm classes (the ones with __tablename__ in the class). And for example the BaseOperator to the operator packages. I've created a lot of tickets to move the classes one by one out of models.py. The reason to do this one by one is to relieve the pain of fixing the circular dependencies. Refactor: Move DagBag out of models.py https://issues.apache.org/jira/browse/AIRFLOW-3456 Refactor: Move User out of models.py https://issues.apache.org/jira/browse/AIRFLOW-3457 Refactor: Move Connection out of models.py https://issues.apache.org/jira/browse/AIRFLOW-3458 Refactor: Move DagPickle out of models.py https://issues.apache.org/jira/browse/AIRFLOW-3459 Refactor: Move TaskInstance out of models.py https://issues.apache.org/jira/browse/AIRFLOW-3460 Refactor: Move TaskFail out of models.py https://issues.apache.org/jira/browse/AIRFLOW-3461 Refactor: Move TaskReschedule out of models.py https://issues.apache.org/jira/browse/AIRFLOW-3462 Refactor: Move Log out of models.py https://issues.apache.org/jira/browse/AIRFLOW-3463 Refactor: Move SkipMixin out of models.py https://issues.apache.org/jira/browse/AIRFLOW-3464 Refactor: Move BaseOperator out of models.py https://issues.apache.org/jira/browse/AIRFLOW-3465 Refactor: Move DAG out of models.py https://issues.apache.org/jira/browse/AIRFLOW-3466 Refactor: Move Chart out of models.py https://issues.apache.org/jira/browse/AIRFLOW-3467 Refactor: Move KnownEventType out of models.py https://issues.apache.org/jira/browse/AIRFLOW-3468 Refactor: Move KnownEvent out of models.py https://issues.apache.org/jira/browse/AIRFLOW-3469 Refactor: Move Variable out of models.py https://issues.apache.org/jira/browse/AIRFLOW-3470 Refactor: Move XCom out of models.py https://issues.apache.org/jira/browse/AIRFLOW-3471 Refactor: Move DagStat out of models.py https://issues.apache.org/jira/browse/AIRFLOW-3472 Refactor: Move DagRun out of models.py https://issues.apache.org/jira/browse/AIRFLOW-3473 Refactor: Move SlaMiss out of models.py https://issues.apache.org/jira/browse/AIRFLOW-3474 Refactor: Move ImportError out of models.py https://issues.apache.org/jira/browse/AIRFLOW-3475 Refactor: Move KubeResourceVersion out of models.py https://issues.apache.org/jira/browse/AIRFLOW-3476 Refactor: Move KubeWorkerIdentifier out of models.py https://issues.apache.org/jira/browse/AIRFLOW-3477 Some classes are really simple, and would also be a nice opportunity for newcomers to start contributing to Airflow :-) Cheers, Fokko
