git.net

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

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 <fengtao04@xxxxxxxxx<mailto:fengtao04@xxxxxxxxx>> 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.koeltringer@xxxxxxxxx<mailto:andreas.koeltringer@xxxxxxxxx>> 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 <something>`

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 <fokko@xxxxxxxxxxxxxx<mailto:fokko@xxxxxxxxxxxxxx>>
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.koeltringer@xxxxxxxxx<mailto:andreas.koeltringer@xxxxxxxxx>