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 <[email protected]>: > 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 <[email protected]> > 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 <[email protected]<mailto: > [email protected]>> 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 < > >> [email protected]<mailto:[email protected]>> > 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 <[email protected] > <mailto:[email protected]>> > >> 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: [email protected]<mailto: > [email protected]> > >> > >> > >> > >> > >> > >> > > >
