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

Re: Refactor

Andreas good idea, without that the only way to refactor is a big bang all at once.

I made a start and renamed to /airflow/models/ and moved class Connection to /airflow/models/ (AIRFLOW-3458). PR is here: 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/ 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:


a suggestion to make this process easier:

a folder could be created called `models`. The `` could then
into that folder and renamed to ``. That way, all the other
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
into separate files. The 'moved' class then needs to be imported in
`` - to not affect the rest of airflow.

Example: move class `User` to `models/`. In `models/`
`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
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
have airflow.models.connection module containing just a Connection class
for example?


On 6 Dec 2018, at 11:35, Driesprong, Fokko <fokko@xxxxxxxxxxxxxx<mailto:fokko@xxxxxxxxxxxxxx>>

Hi All,

I think it is time to refactor the infamous This file is far
big, and it only keeps growing. My suggestion is to create a new
called models, which will contain all the orm classes (the ones
with __tablename__ in the class). And for example the BaseOperator to
operator packages. I've created a lot of tickets to move the classes
one out of The reason to do this one by one is to relieve
pain of fixing the circular dependencies.

Refactor: Move DagBag out of

Refactor: Move User out of

Refactor: Move Connection out of

Refactor: Move DagPickle out of

Refactor: Move TaskInstance out of

Refactor: Move TaskFail out of

Refactor: Move TaskReschedule out of

Refactor: Move Log out of

Refactor: Move SkipMixin out of

Refactor: Move BaseOperator out of

Refactor: Move DAG out of

Refactor: Move Chart out of

Refactor: Move KnownEventType out of

Refactor: Move KnownEvent out of

Refactor: Move Variable out of

Refactor: Move XCom out of

Refactor: Move DagStat out of

Refactor: Move DagRun out of

Refactor: Move SlaMiss out of

Refactor: Move ImportError out of

Refactor: Move KubeResourceVersion out of

Refactor: Move KubeWorkerIdentifier out of

Some classes are really simple, and would also be a nice opportunity
newcomers to start contributing to Airflow :-)

Cheers, Fokko

Andreas Koeltringer
Mail:   andreas.koeltringer@xxxxxxxxx<mailto:andreas.koeltringer@xxxxxxxxx>