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]>
>> 
>> 
>> 
>> 
>> 
>> 
> 

Reply via email to