You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@airflow.apache.org by Jeremiah Lowin <jl...@apache.org> on 2016/06/02 13:03:15 UTC
Splitting models.py
Models.py is becoming monolithic. We've discussed refactoring it many
times. No way around this: refactoring it will suck. It will break PRs and
require rebases. It will make it impossible to see diffs.
On the other hand it will make future changes much more manageable. It will
implicitly address concerns about PRs that touch "core" areas because we'll
be able to see if "dags.py" is altered, as opposed to "xcom.py". It will
make the codebase more digestible and clear.
I'm not exactly lining up to champion this but it will only get harder and
harder to do so I want to raise the issue to the list...
J
Re: Splitting models.py
Posted by Chris Riccomini <cr...@apache.org>.
@jeremiah, perhaps open a JIRA and tag it as Airflow 2.0, so we don't lose
track?
On Thu, Jun 2, 2016 at 1:46 PM, Jeremiah Lowin <jl...@apache.org> wrote:
> That all makes sense to me. I knew I wouldn't win any popularity contests
> by bringing this up but I think we should keep it in mind in case it
> becomes more feasible for any reason.
>
> On Thu, Jun 2, 2016 at 3:21 PM Chris Riccomini <cr...@apache.org>
> wrote:
>
> > I agree with what Arthur/Maxime said.
> >
> > On Thu, Jun 2, 2016 at 11:32 AM, Maxime Beauchemin <
> > maximebeauchemin@gmail.com> wrote:
> >
> > > I think it's possible to do it without breaking the API by frontloading
> > > things in `models/__init__.py`, but there are way so many things in
> > flight
> > > at the moment that to me this effort qualifies as low-ish value and
> high
> > > complexity.
> > >
> > > Cycles spent refactoring the tests would be a much better investment
> from
> > > my perspective.
> > >
> > > Max
> > >
> > > On Thu, Jun 2, 2016 at 9:54 AM, Arthur Wiedmer <ar...@apache.org>
> > wrote:
> > >
> > > > A final observation, since this may include breaking changes, should
> we
> > > > target these large refactors for 2.0 rather than 1.8?
> > > >
> > > > I agree that these are important changes, but maybe getting our feet
> > wet
> > > > with an apache release or two to settle on a release process (+
> testing
> > > > infrastructure less dependent on Airbnb) before breaking too much
> stuff
> > > > might not be a bad idea.
> > > >
> > > > Best,
> > > > Arthur
> > > >
> > > > On Thu, Jun 2, 2016 at 9:50 AM, Arthur Wiedmer <ar...@apache.org>
> > > wrote:
> > > >
> > > > > I'd love to do it. Actually this + refactoring the core.py tests
> > would
> > > be
> > > > > amazing.
> > > > >
> > > > > But the amount of havok to fix stuff afterwards, including
> temporary
> > > > > compatibility adjustments would require maybe a temporary lock of
> > quiet
> > > > > time on the models. It is hard to catch all of the added changes in
> > the
> > > > > rebases.
> > > > >
> > > > > Should we merge the remaining few Scheduler PRs first and then do
> the
> > > > > refactor?
> > > > >
> > > > > Best,
> > > > > Arthur
> > > > >
> > > > > On Thu, Jun 2, 2016 at 6:03 AM, Jeremiah Lowin <jl...@apache.org>
> > > > wrote:
> > > > >
> > > > >> Models.py is becoming monolithic. We've discussed refactoring it
> > many
> > > > >> times. No way around this: refactoring it will suck. It will break
> > PRs
> > > > and
> > > > >> require rebases. It will make it impossible to see diffs.
> > > > >>
> > > > >> On the other hand it will make future changes much more
> manageable.
> > It
> > > > >> will
> > > > >> implicitly address concerns about PRs that touch "core" areas
> > because
> > > > >> we'll
> > > > >> be able to see if "dags.py" is altered, as opposed to "xcom.py".
> It
> > > will
> > > > >> make the codebase more digestible and clear.
> > > > >>
> > > > >> I'm not exactly lining up to champion this but it will only get
> > harder
> > > > and
> > > > >> harder to do so I want to raise the issue to the list...
> > > > >>
> > > > >> J
> > > > >>
> > > > >
> > > > >
> > > >
> > >
> >
>
Re: Splitting models.py
Posted by Jeremiah Lowin <jl...@apache.org>.
That all makes sense to me. I knew I wouldn't win any popularity contests
by bringing this up but I think we should keep it in mind in case it
becomes more feasible for any reason.
On Thu, Jun 2, 2016 at 3:21 PM Chris Riccomini <cr...@apache.org>
wrote:
> I agree with what Arthur/Maxime said.
>
> On Thu, Jun 2, 2016 at 11:32 AM, Maxime Beauchemin <
> maximebeauchemin@gmail.com> wrote:
>
> > I think it's possible to do it without breaking the API by frontloading
> > things in `models/__init__.py`, but there are way so many things in
> flight
> > at the moment that to me this effort qualifies as low-ish value and high
> > complexity.
> >
> > Cycles spent refactoring the tests would be a much better investment from
> > my perspective.
> >
> > Max
> >
> > On Thu, Jun 2, 2016 at 9:54 AM, Arthur Wiedmer <ar...@apache.org>
> wrote:
> >
> > > A final observation, since this may include breaking changes, should we
> > > target these large refactors for 2.0 rather than 1.8?
> > >
> > > I agree that these are important changes, but maybe getting our feet
> wet
> > > with an apache release or two to settle on a release process (+ testing
> > > infrastructure less dependent on Airbnb) before breaking too much stuff
> > > might not be a bad idea.
> > >
> > > Best,
> > > Arthur
> > >
> > > On Thu, Jun 2, 2016 at 9:50 AM, Arthur Wiedmer <ar...@apache.org>
> > wrote:
> > >
> > > > I'd love to do it. Actually this + refactoring the core.py tests
> would
> > be
> > > > amazing.
> > > >
> > > > But the amount of havok to fix stuff afterwards, including temporary
> > > > compatibility adjustments would require maybe a temporary lock of
> quiet
> > > > time on the models. It is hard to catch all of the added changes in
> the
> > > > rebases.
> > > >
> > > > Should we merge the remaining few Scheduler PRs first and then do the
> > > > refactor?
> > > >
> > > > Best,
> > > > Arthur
> > > >
> > > > On Thu, Jun 2, 2016 at 6:03 AM, Jeremiah Lowin <jl...@apache.org>
> > > wrote:
> > > >
> > > >> Models.py is becoming monolithic. We've discussed refactoring it
> many
> > > >> times. No way around this: refactoring it will suck. It will break
> PRs
> > > and
> > > >> require rebases. It will make it impossible to see diffs.
> > > >>
> > > >> On the other hand it will make future changes much more manageable.
> It
> > > >> will
> > > >> implicitly address concerns about PRs that touch "core" areas
> because
> > > >> we'll
> > > >> be able to see if "dags.py" is altered, as opposed to "xcom.py". It
> > will
> > > >> make the codebase more digestible and clear.
> > > >>
> > > >> I'm not exactly lining up to champion this but it will only get
> harder
> > > and
> > > >> harder to do so I want to raise the issue to the list...
> > > >>
> > > >> J
> > > >>
> > > >
> > > >
> > >
> >
>
Re: Splitting models.py
Posted by Chris Riccomini <cr...@apache.org>.
I agree with what Arthur/Maxime said.
On Thu, Jun 2, 2016 at 11:32 AM, Maxime Beauchemin <
maximebeauchemin@gmail.com> wrote:
> I think it's possible to do it without breaking the API by frontloading
> things in `models/__init__.py`, but there are way so many things in flight
> at the moment that to me this effort qualifies as low-ish value and high
> complexity.
>
> Cycles spent refactoring the tests would be a much better investment from
> my perspective.
>
> Max
>
> On Thu, Jun 2, 2016 at 9:54 AM, Arthur Wiedmer <ar...@apache.org> wrote:
>
> > A final observation, since this may include breaking changes, should we
> > target these large refactors for 2.0 rather than 1.8?
> >
> > I agree that these are important changes, but maybe getting our feet wet
> > with an apache release or two to settle on a release process (+ testing
> > infrastructure less dependent on Airbnb) before breaking too much stuff
> > might not be a bad idea.
> >
> > Best,
> > Arthur
> >
> > On Thu, Jun 2, 2016 at 9:50 AM, Arthur Wiedmer <ar...@apache.org>
> wrote:
> >
> > > I'd love to do it. Actually this + refactoring the core.py tests would
> be
> > > amazing.
> > >
> > > But the amount of havok to fix stuff afterwards, including temporary
> > > compatibility adjustments would require maybe a temporary lock of quiet
> > > time on the models. It is hard to catch all of the added changes in the
> > > rebases.
> > >
> > > Should we merge the remaining few Scheduler PRs first and then do the
> > > refactor?
> > >
> > > Best,
> > > Arthur
> > >
> > > On Thu, Jun 2, 2016 at 6:03 AM, Jeremiah Lowin <jl...@apache.org>
> > wrote:
> > >
> > >> Models.py is becoming monolithic. We've discussed refactoring it many
> > >> times. No way around this: refactoring it will suck. It will break PRs
> > and
> > >> require rebases. It will make it impossible to see diffs.
> > >>
> > >> On the other hand it will make future changes much more manageable. It
> > >> will
> > >> implicitly address concerns about PRs that touch "core" areas because
> > >> we'll
> > >> be able to see if "dags.py" is altered, as opposed to "xcom.py". It
> will
> > >> make the codebase more digestible and clear.
> > >>
> > >> I'm not exactly lining up to champion this but it will only get harder
> > and
> > >> harder to do so I want to raise the issue to the list...
> > >>
> > >> J
> > >>
> > >
> > >
> >
>
Re: Splitting models.py
Posted by Maxime Beauchemin <ma...@gmail.com>.
I think it's possible to do it without breaking the API by frontloading
things in `models/__init__.py`, but there are way so many things in flight
at the moment that to me this effort qualifies as low-ish value and high
complexity.
Cycles spent refactoring the tests would be a much better investment from
my perspective.
Max
On Thu, Jun 2, 2016 at 9:54 AM, Arthur Wiedmer <ar...@apache.org> wrote:
> A final observation, since this may include breaking changes, should we
> target these large refactors for 2.0 rather than 1.8?
>
> I agree that these are important changes, but maybe getting our feet wet
> with an apache release or two to settle on a release process (+ testing
> infrastructure less dependent on Airbnb) before breaking too much stuff
> might not be a bad idea.
>
> Best,
> Arthur
>
> On Thu, Jun 2, 2016 at 9:50 AM, Arthur Wiedmer <ar...@apache.org> wrote:
>
> > I'd love to do it. Actually this + refactoring the core.py tests would be
> > amazing.
> >
> > But the amount of havok to fix stuff afterwards, including temporary
> > compatibility adjustments would require maybe a temporary lock of quiet
> > time on the models. It is hard to catch all of the added changes in the
> > rebases.
> >
> > Should we merge the remaining few Scheduler PRs first and then do the
> > refactor?
> >
> > Best,
> > Arthur
> >
> > On Thu, Jun 2, 2016 at 6:03 AM, Jeremiah Lowin <jl...@apache.org>
> wrote:
> >
> >> Models.py is becoming monolithic. We've discussed refactoring it many
> >> times. No way around this: refactoring it will suck. It will break PRs
> and
> >> require rebases. It will make it impossible to see diffs.
> >>
> >> On the other hand it will make future changes much more manageable. It
> >> will
> >> implicitly address concerns about PRs that touch "core" areas because
> >> we'll
> >> be able to see if "dags.py" is altered, as opposed to "xcom.py". It will
> >> make the codebase more digestible and clear.
> >>
> >> I'm not exactly lining up to champion this but it will only get harder
> and
> >> harder to do so I want to raise the issue to the list...
> >>
> >> J
> >>
> >
> >
>
Re: Splitting models.py
Posted by Arthur Wiedmer <ar...@apache.org>.
A final observation, since this may include breaking changes, should we
target these large refactors for 2.0 rather than 1.8?
I agree that these are important changes, but maybe getting our feet wet
with an apache release or two to settle on a release process (+ testing
infrastructure less dependent on Airbnb) before breaking too much stuff
might not be a bad idea.
Best,
Arthur
On Thu, Jun 2, 2016 at 9:50 AM, Arthur Wiedmer <ar...@apache.org> wrote:
> I'd love to do it. Actually this + refactoring the core.py tests would be
> amazing.
>
> But the amount of havok to fix stuff afterwards, including temporary
> compatibility adjustments would require maybe a temporary lock of quiet
> time on the models. It is hard to catch all of the added changes in the
> rebases.
>
> Should we merge the remaining few Scheduler PRs first and then do the
> refactor?
>
> Best,
> Arthur
>
> On Thu, Jun 2, 2016 at 6:03 AM, Jeremiah Lowin <jl...@apache.org> wrote:
>
>> Models.py is becoming monolithic. We've discussed refactoring it many
>> times. No way around this: refactoring it will suck. It will break PRs and
>> require rebases. It will make it impossible to see diffs.
>>
>> On the other hand it will make future changes much more manageable. It
>> will
>> implicitly address concerns about PRs that touch "core" areas because
>> we'll
>> be able to see if "dags.py" is altered, as opposed to "xcom.py". It will
>> make the codebase more digestible and clear.
>>
>> I'm not exactly lining up to champion this but it will only get harder and
>> harder to do so I want to raise the issue to the list...
>>
>> J
>>
>
>
Re: Splitting models.py
Posted by Arthur Wiedmer <ar...@apache.org>.
I'd love to do it. Actually this + refactoring the core.py tests would be
amazing.
But the amount of havok to fix stuff afterwards, including temporary
compatibility adjustments would require maybe a temporary lock of quiet
time on the models. It is hard to catch all of the added changes in the
rebases.
Should we merge the remaining few Scheduler PRs first and then do the
refactor?
Best,
Arthur
On Thu, Jun 2, 2016 at 6:03 AM, Jeremiah Lowin <jl...@apache.org> wrote:
> Models.py is becoming monolithic. We've discussed refactoring it many
> times. No way around this: refactoring it will suck. It will break PRs and
> require rebases. It will make it impossible to see diffs.
>
> On the other hand it will make future changes much more manageable. It will
> implicitly address concerns about PRs that touch "core" areas because we'll
> be able to see if "dags.py" is altered, as opposed to "xcom.py". It will
> make the codebase more digestible and clear.
>
> I'm not exactly lining up to champion this but it will only get harder and
> harder to do so I want to raise the issue to the list...
>
> J
>
Re: Splitting models.py
Posted by Bolke de Bruin <bd...@gmail.com>.
Mmm it does look like a "how to screw yourself" email :-).
A refactor would be welcome. If we are going to break PRs why not do a "add license" exercise and "replace high charts" exercise in one go?
B
Sent from my iPhone
> On 2 jun. 2016, at 15:03, Jeremiah Lowin <jl...@apache.org> wrote:
>
> Models.py is becoming monolithic. We've discussed refactoring it many
> times. No way around this: refactoring it will suck. It will break PRs and
> require rebases. It will make it impossible to see diffs.
>
> On the other hand it will make future changes much more manageable. It will
> implicitly address concerns about PRs that touch "core" areas because we'll
> be able to see if "dags.py" is altered, as opposed to "xcom.py". It will
> make the codebase more digestible and clear.
>
> I'm not exactly lining up to champion this but it will only get harder and
> harder to do so I want to raise the issue to the list...
>
> J