airflow-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From "Driesprong, Fokko" <fo...@driesprong.frl>
Subject Re: Refactor models.py
Date Sun, 30 Dec 2018 14:58:09 GMT
Hi Beau,

We should split the tests from tests/modes.py as well. Breaking backward
compatibility is possible, but then it would be nice to first have a
version where we thrown a deprecation warning.

Cheers, Fokko

Op zo 30 dec. 2018 om 05:22 schreef Beau Barker <beauinmelbourne@gmail.com>:

> Yes it should be named after the module it's testing.
>
> Regarding backwards compatibility, isn't a new major version a chance to
> break backwards compatibility?
>
>
> > On 30 Dec 2018, at 3:24 am, Stefan Seelmann <mail@stefan-seelmann.de>
> wrote:
> >
> > When moving a class out into its own file should the corresponding tests
> > also be moved out of tests/models.py?
> >
> > I'd say yes, that file already has more then 3200 lines.
> >
> > Kind Regards,
> > Stefan
> >
> >> On 12/17/18 8:05 PM, Bas Harenslak wrote:
> >> 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@gmail.com<mailto:
> fengtao04@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.koeltringer@n-fuse.co<mailto:andreas.koeltringer@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 <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@driesprong.frl
> <mailto:fokko@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.koeltringer@n-fuse.co<mailto:
> andreas.koeltringer@n-fuse.co>
> >>
> >>
> >>
> >>
> >>
> >>
> >
>

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