+1 though I'd recommend doing a review / merge / PR cleanup blitz prior to starting the file splitting
Max On Wed, Mar 14, 2018 at 11:34 PM, Tao Feng <fengta...@gmail.com> wrote: > +1 on splitting the long file. > > > On Wed, Mar 14, 2018 at 5:50 PM, Marc Bollinger <m...@lumoslabs.com> > wrote: > > > +1 > > > > This was one of the first things that struck us when integrating Airflow, > > but as we aren't (weren't?) pythonist[a]s at the time, didn't necessarily > > feel comfortable chiming in with an Issue. > > > > On Wed, Mar 14, 2018 at 2:57 PM, George Leslie-Waksman < > > geo...@cloverhealth.com.invalid> wrote: > > > > > This has been something that I've wanted to see for a while but as > Fokko > > > mentions, it can be hard for very large files that see lots of PRs. > > > Probably easiest is to pull them apart one class at a time to minimize > > > merge conflict risks. > > > > > > Additionally, in some cases, there are interdependencies that may make > it > > > hard to pull them apart without creating circular dependencies, > > especially > > > in the case of models.py > > > > > > Either way, I am hugely in favor of trying to pull these things apart. > > > > > > --George > > > > > > On Sun, Mar 11, 2018 at 1:44 PM Driesprong, Fokko <fo...@driesprong.frl > > > > > wrote: > > > > > > > Maybe wait until this one has been merged 🤪 > > > > https://github.com/apache/incubator-airflow/pull/3116/files > > > > > > > > Cheers, Fokko > > > > > > > > Op zo 11 mrt. 2018 om 21:34 schreef Driesprong, Fokko > > > <fo...@driesprong.frl > > > > > > > > > > > > > > Hi Bruno, > > > > > > > > > > I agree that there are files that are too big as you mentioned. > > > Sometimes > > > > > structures are being refactored like I did with the sensors. > > > > > > > > > > The problem is that if you refactor one of these files, most PR > > > requests > > > > > will have merge conflicts. But this should not be seen as an > > impediment > > > > to > > > > > increase the quality of Airflow’s code. > > > > > Especially the models.py should be fairly easy to refactor and > split > > > into > > > > > different files. This file grew due historical reasons. If you feel > > > like > > > > > picking up this task, I encourage you to create a Jira for it and > > start > > > > > cracking. > > > > > > > > > > Cheers, Fokko > > > > > > > > > > Op zo 11 mrt. 2018 om 20:11 schreef Bruno Bonagura < > > > bbonagu...@gmail.com > > > > > > > > > > > > > > >> I mean 'big files into submodules'. > > > > >> > > > > >> On Sun, Mar 11, 2018 at 4:09 PM, Bruno Bonagura < > > bbonagu...@gmail.com > > > > > > > > >> wrote: > > > > >> > > > > >> > Hello, > > > > >> > > > > > >> > I'm pretty newbie to the project, so I apologize in advance if > I'm > > > > being > > > > >> > too silly. > > > > >> > > > > > >> > Is there any plan or goal to refactor big files into packages, > > like > > > it > > > > >> > happened to sensors.py? Has it been tried before with the files > I > > > list > > > > >> > bellow and failed? I searched Jira for 'refactor' and didn't > find > > > > much. > > > > >> > > > > > >> > Airflow codebase has some giant ugly files, what makes it > > difficult > > > to > > > > >> > find things and organize. Also it makes difficult for new > > > contributors > > > > >> to > > > > >> > understand the code and could even keep some from contributing, > > > having > > > > >> > impact in the community growth. > > > > >> > > > > > >> > Here are the bigger files: > > > > >> > > > > > >> > 5008 airflow/models.py > > > > >> > 3121 tests/jobs.py > > > > >> > 2844 airflow/www/views.py > > > > >> > 2595 airflow/jobs.py > > > > >> > 2494 tests/core.py > > > > >> > 1847 tests/models.py > > > > >> > 1674 airflow/bin/cli.py > > > > >> > 1487 airflow/contrib/hooks/bigquery_hook.py > > > > >> > 1045 airflow/contrib/operators/dataproc_operator.py > > > > >> > > > > > >> > I've been playing and experimenting with models.py a little. > It's > > > > >> > difficult, but it might be possible. > > > > >> > > > > > >> > Well, that's it. > > > > >> > > > > > >> > Best regards. > > > > >> > Bruno > > > > >> > > > > > >> > > > > > > > > > > > > > > >
