Re: [Patch] PGAdmin 4 JSON Handling - Mailing list pgadmin-hackers

Le jeudi 16 avril 2015 15:32:18 Dave Page a écrit :
> On Thu, Apr 16, 2015 at 2:44 PM, Ronan Dunklau <ronan.dunklau@dalibo.com>
wrote:
> > Le jeudi 16 avril 2015 11:40:14 Dave Page a écrit :
> >> On Thu, Apr 16, 2015 at 11:19 AM, Ronan Dunklau
> >>
> >> <ronan.dunklau@dalibo.com> wrote:
> >> > Le jeudi 16 avril 2015 15:46:51 Ashesh Vashi a écrit :
> >> >> Hi Ronan,
> >> >>
> >> >> On Thu, Apr 16, 2015 at 2:49 PM, Ronan Dunklau
> >> >> <ronan.dunklau@dalibo.com>
> >> >>
> >> >> wrote:
> >> >> > Hello.
> >> >> >
> >> >> > I'm trying to dive into the PGAdmin 4 codebase, and one low hanging
> >> >> > fruit
> >> >> > that
> >> >> > I saw was the handling of JSON data.
> >> >> >
> >> >> > The first patch is really trivial, and allows PgAdmin4 to run on
> >> >> > system
> >> >> > with
> >> >> > case sensitive filesystems.
> >> >> >
> >> >> > JSON data should be returned to the client using an application/json
> >> >> > MIME-
> >> >> > TYPE. Flask already provides an easy way to generate JSON response,
> >> >> > with
> >> >> > its
> >> >> > jsonify function.
> >> >> >
> >> >> > This patch does not change anything architecturally, it just ensure
> >> >> > that
> >> >> > the
> >> >> > get_nodes method of each hook returns JSONizable objects, instead of
> >> >> > building
> >> >> > them manually.
> >> >> >
> >> >> > Moreover, there was a function already in place to return JSON
> >> >> > document
> >> >> > according to a certain layout (everything under "data", with
> >> >> > metadata
> >> >> > attached
> >> >> > along the way), which was not used by the ACi tree. This patch also
> >> >> > changes
> >> >> > this format to ensure the json responses returned by the application
> >> >> > are
> >> >> > consistent.
> >> >>
> >> >> Thanks for the patches.
> >> >> These changes are already been done in my current development work,
> >> >> which
> >> >> is still a WIP. (Hence - it has not been checked-in/shared.)
> >> >>
> >> >> Thanks any way.
> >>
> >> Oh, they are?
> >>
> >> > Is there a public branch where I can see this WIP ?
> >>
> >> I assume it's all on his laptop. We (the pgAdmin project) have never
> >> published WIP branches that I can recall, except for GSoC projects.
> >>
> >> > As for the other proposals regarding the structure of the javascript
> >> > code
> >> > and the module / hooks API, do you have any comment ?
> >>
> >> It's been on my personal TODO for a little while to sort out that code
> >> into a proper OO class hierarchy as you suggest. Is that something you
> >> would like to take a look at Ronan? I won't be touching it for quite a
> >> while as I'm overloaded at the moment (which is why I asked Ashesh to
> >> help out).
> >
> > Ok, I'm in the process of doing just that, I'll post a patch as soon as I
> > have something.
>
> Cool, thanks.

Please find attached a WIP patch "reifying" the module API.

The main browser/server_groups/server structure has been converted to his API,
whereas the test, help and about modules have not.

The following changes were made:

 - instead of relying on a convention (views/hooks) for the module, each
module should now inherit from PgAdminModule instead of the classical Flask
BluePrint.

This class brings several methods:
 -  get_own_stylesheets, which returns the stylesheets used by the module
(excluding its submodules stylesheets)
  - get_own_javascripts
  - menu_items, which returns a dictionray mapping the old hook names
(context_items etc) to a list of MenuItem instances

For more specialized modules (as for now, any module that should be part of
the browser tree construction), one can define an abstract base class defining
additional methods.

For example, the BrowserPluginModule abstract base class defines the following
methods:

   - jssnippets
   - csssnipeets
   - node_type
   - get_nodes

Once again, for modules supposed to get included under the servergroup
modules, a new base class is declared, overloading the get_nodes method by
another taking a server_group as an argument.

Sorry for the wall of text, but...

During the development of this patch, I encountered several things which got
me worried:

 - all the client-side interface building relies on global functions, and on
snippets of javsacript generated by the server. I feel like this apporach is
extremely fragile. Even before the patch, most of the tree actions were not
working properly.
 - the other downside of this approach is that the synchronisation between
client and server state is done manually. It leads to hard to debug errors,
where a node tree does not appear at the right place on the tree until the
whole page is reloaded
 - the various components integration (docker, acitree are the main ones I
noticed) feel once again not really robust. All this widget assembly is done
"ad-hoc".
 - the CRUD functionality should be abstracted away. There is no reason to
declare  "add/rename/drop" menuentries, as well as their corresponding
enpoints manually. This is once again error-prone, and should prove to be hard
to maintain in the future.


I'm not sure I understand the architecture of the whole app fully, so please
correct me if the following is erroneous.

The application seems to sit somewhere between a "classical" web applicaiton,
with content generated by the server and a "rich application", with "one page"
containing the whole application and its states managed by Javascript.  I
think this leads to a poor separation of concerns between the client and the
server, and the boundary between the two should be more clearly expressed.

In my opinion, porting a desktop app to the browser should lend itself
naturally to a single page design, with the server merely acting as a
datastore returning json documents and static files. As such, frameworks
intended for this kind of application should be considered. I'm thinking about
two different approaches, but I'm not really familiar enough with those
technologies to recommend one or the other:
   - using  a "structural" framework, providing an MVC base, like Backbone,
Ember or any of their counterparts. It should bring us  the structure of the
application, by allowing us to dice the javascript code into various modules /
submodules and helping us implement a proper separation of concern between
what is a view , a model etc...
  - using a rich javascript application framework, like the  dojo toolkit or
extjs. I used to be familiar with dojo toolkit, a bit less with ext, which has
licensing concerns anyway.

There are probablly a ton more options, but think we should start asking
those questions to see where this takes us. I don't see the code base as it is
able to grow to the scope of the project.


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