Re: PGAdmin 4 architecture (Was: [Patch] PGAdmin 4 JSON Handling)

Le jeudi 23 avril 2015 11:44:49 Dave Page a écrit :
> On Thu, Apr 23, 2015 at 9:18 AM, Ashesh Vashi <ashesh.vashi@enterprisedb.com
> > wrote:
> >
> >
> > On Thu, Apr 23, 2015 at 12:52 PM, Ronan Dunklau <ronan.dunklau@dalibo.com>
> >
> > wrote:
> >>  Le jeudi 23 avril 2015 10:53:48 Ashesh Vashi a écrit :
> >> > Hi Dave/Ronan,
> >> >
> >> > On Mon, Apr 20, 2015 at 4:10 PM, Dave Page <dpage@pgadmin.org> wrote:
> >> > > On Mon, Apr 20, 2015 at 10:52 AM, Ronan Dunklau
> >> > >
> >> > > <ronan.dunklau@dalibo.com> wrote:
> >> > > >> Ronan; can you update the test, help and about modules as well
> >>
> >> please?
> >>
> >> > > > Done, please find attached a new patch for that. Ashesh, once
> >> > > > you're
> >> > >
> >> > > done with
> >> > >
> >> > > > what you are doing now, feel free to ask me for any help needed to
> >> > >
> >> > > integrate
> >> > >
> >> > > > this after the fact.
> >> > >
> >> > > Thanks.
> >> >
> >> > Thanks - It looks good to me.
> >> >
> >> > (And, attached patch is based on that.)
> >>
> >> Maybe I'm missing something, but it seems like a "generate_browser_node"
> >> function is missing:
> >>
> >>
> >>
> >> File
> >> "/home/ro/projets/pgadmin4/web/pgadmin/browser/server_groups/__init__.py"
> >> ,
> >> line 21, in <module>
> >>
> >> from pgadmin.browser.utils import generate_browser_node
> >>
> >> ImportError: cannot import name generate_browser_node
> >>
> >>
> >>
> >>
> >>
> >> It also brought to my attention that I should have removed the only
> >> function (register_modules) from pgadmin.browser.utils.
> >
> > Yeah - it is missing.
> > Please find a separate patch for the utils.
> >
> > Actually - you removed the web/pgadmin/browser/utils.py, hence - when I
> > made the patch on top of yours, git detected the file as new file, and did
> > not include the diff for it.
> > The attached patch is only for utils file.
>
> Hi
>
> I can't persuade these patches to apply to my tree. When you get a minute,
> can you please send me a complete 'git diff' of everything from HEAD?
>
> Thanks.

Here is the diff from my tree after applying both Ashesh's patches on top of
mine.

As for the json handling, what do you think about using standard http codes ?
It feels a bit strange to receive a 200 when the operation could not be
completed.

For example, right now, adding a server returns a "missing required parameter
(host)." That should return a 400 (Bad Request) in my opinion, with the
detailed error message in the response.

--
Ronan Dunklau
http://dalibo.com - http://dalibo.org