[PATCH 16/51] settings: Add Django REST framework to the project

[Finucane, Stephen]

> Hi,
> 
> On Tue, Sep 22, 2015 at 01:19:25AM +0100, Finucane, Stephen wrote:
> > > Let's try to do have a more modern approach to this web thing. Having a
> > > separate API returning some JSON ensures a nice split between data and
> > > UI. This API can be both used withing the HTML pages, loading/updating
> > > data with AJAX, and from any other kind of client (say, it could
> replace
> > > the XML-RPC API patchwork currently has).
> > >
> > > But for now, let's experiment on Series.
> > >
> > > Signed-off-by: Damien Lespiau <damien.lespiau at intel.com>
> >
> > -1 This series revolves around adding support for series parsing: this
> > patch (and subsequent, related patches) focuses on adding a different,
> > large and currently unrequired feature. This secondary feature should
> > be done in a different series, as suggested previously:
> >
> >   https://lists.ozlabs.org/pipermail/patchwork/2015-August/001417.html
> 
> Doing so in a different series would mean redoing the client-side part
> of Series. If we can agree to move to a REST API long term, I think
> having the REST API as a dependency for Series fine.

No, it's not. REST API is a different feature to Series support. These need to be done (and reviewed) separately.

> > Addressing your points from that previous mail:
> >
> > > Of one of the main thing is being able to re-use the API entry points
> > > from the web page itself (say using jquery's ajax). That makes the UI
> > > much nicer to use and reduces what the server needs to do (no full page
> > > reloads).
> >
> > AJAX is great, but it's not necessary for series support. I'm all for
> > responsive applications (in all senses), but I have yet to see a
> > complaint about patchwork's resource demands nor a user requesting the
> > functionality that AJAX would provide. If you personally think
> > patchwork then by all means, make it happen but do so in another
> > series.
> 
> I'd rather not spend time spinning patches if the end goal is the same.

I understand, but do you expect people to voluntarily review a 51 patch series that adds three different features?

> > > It also forces us to think in terms of objects and methods and have a
> > > clear separation between the backend and frontend(s).
> >
> > What does this give us, besides slightly nicer "architecture"? The end
> > user won't see this, but they will see the bugs and design oversights
> > that will inevitably be present. APIs are hard work and once they're
> > out there there's no going back: we want to make sure the one we
> > implement is "right". This is something that needs to be discussed in
> > its own thread.
> 
> I'm fine with discussing the API anywhere. I'm not saying that it's
> perfect and in fact I've already done a few adjustments locally. There
> are ways to mature an API before declaring it stable, especially when
> the number of users is rather small and we have pwclient for the stable
> part.

No. An API is a contract. We need to abide by this contract. API versioning exists for a reason.

> > > Right now the
> > > XML-RPC API feels like a bunch of disjointed functions. We also mix the
> > > views with API and have a number of "random" API entry points as POSTS
> > > requests on view.
> >
> > Now this is a real issue. However, why not fix this in another series
> > rather than trying to replace something that is well working but
> > perhaps not as clean as it could be? The XML-RPC API may need some
> > work but there's nothing fundamentally wrong with it (RPC is not dead:
> > it's just out of fashion).
> 
> > In summary, while I understand what you're trying I think you're going
> > about it the wrong way. There may be some valid reasons for using
> > REST, but we are currently able to do patch support with XML-RPC and
> > we can sure do series support the same way :)
> 
> So.
> 
> 1/ Are you happy with the idea of moving to a REST API instead of
> XML-RPC?
> 
> If No, that makes things easy, I'll just fork.

I'm not convinced so we should discuss it. This deserves its own discussion.

> 2/ If yes, then we need a path to make it happen. I didn't think that
> doing so for a new feature was out of line. As always, everything stays
> the same, all the XML-RPC API is still there and pwclient works. We can
> still have the API discussion, of course.
>
> 3/ On top of that, I don't really have the time to redo things
> endlessly. I already have the impression that the re-design series is
> being bikeshedded.

Two revisions of a 51 patch series is not "redo[ing] things endless", Damien.

My complaint here is that this series is too big and muddled to be reviewable. A series should add one feature so it can be reviewed effectively. Let's work on getting series support, which has very well defined use cases, in first. I can help with extending the XML-RPC API if you're tight on time. After that (or in parallel), we can discuss the REST API and what it will give us over the XML-RPC.

> I have the growing feeling that I'll need to fork the project to reach
> my goals in a bounded time. The re-design part is already 1 year old for
> instance.

I'm trying my very best here. Perhaps you could send smaller patch series for a single features and people might have an easier time reviewing things?

Stephen