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

From: Ronan Dunklau <ronan(dot)dunklau(at)dalibo(dot)com>
To: pgadmin-hackers(at)postgresql(dot)org
Cc: Dave Page <dpage(at)pgadmin(dot)org>, Ashesh Vashi <ashesh(dot)vashi(at)enterprisedb(dot)com>
Subject: Re: PGAdmin 4 architecture (Was: [Patch] PGAdmin 4 JSON Handling)
Date: 2015-04-28 11:21:12
Message-ID: 286058824.dFD7emUuSJ@ronan.dunklau.fr
Views: Raw Message | Whole Thread | Download mbox | Resend email
Thread:
Lists: pgadmin-hackers

Le mardi 28 avril 2015 11:32:05 Dave Page a écrit :
> On Tue, Apr 28, 2015 at 10:28 AM, Ronan Dunklau
>
> <ronan(dot)dunklau(at)dalibo(dot)com> wrote:
> >> Thanks, but I think I'm still missing something. This is your patch
> >> applied to the current HEAD (BTW, please use "git diff" to create
> >> patches - the apply a lot more reliably than whatever you're
currently
> >
> >> doing, which "git apply" just doesn't like):
> > I'm using git diff -u, so patch -p1 should apply it just fine. I'll use
> > plain git diff next time if git apply is what you use.
>
> Thanks!
>
> >> ImportError: cannot import name register_modules
> >
> > I think the problem here might be that you have stale bytecompiled
files,
> > in particular pgadmin/browser/hooks, which should not exist
anymore.
> >
> > find -name "*.pyc" -delete
>
> Ah, yes - that was it.
>
> > When developing, it can be useful to set the env variable
> > PYTHONDONTWRITEBYTECODE to prevent the interpreter from
generating those
> > files.
>
> Cool, thanks for the tip.
>
> >> > 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.
> >>
> >> Agreed. Patches welcome as always :-)
> >
> > I'll wait until these patches and Ashesh work on Backbone to get
merged
> > before submitting anything else. The changes are already invasive.
>
> OK. Unfortunately they also seem to have broken a number of things:
>
> - Server icons are no longer displayed on the treeview
>

Fixed

> - Renaming a server fails with:
>
> 2015-04-28 11:25:54,855: INFO werkzeug: 127.0.0.1 - - [28/Apr/2015
> 11:25:54] "PUT /browser/server/obj/1 HTTP/1.1" 500 -
> Traceback (most recent call last):
> File
> "/Users/dpage/.virtualenvs/pgadmin4/lib/python2.7/site-
packages/flask/app.p
> y", line 1836, in __call__
> return self.wsgi_app(environ, start_response)
> File
> "/Users/dpage/.virtualenvs/pgadmin4/lib/python2.7/site-
packages/flask/app.p
> y", line 1820, in wsgi_app
> response = self.make_response(self.handle_exception(e))
> File
> "/Users/dpage/.virtualenvs/pgadmin4/lib/python2.7/site-
packages/flask/app.p
> y", line 1403, in handle_exception
> reraise(exc_type, exc_value, tb)
> File
> "/Users/dpage/.virtualenvs/pgadmin4/lib/python2.7/site-
packages/flask/app.p
> y", line 1817, in wsgi_app
> response = self.full_dispatch_request()
> File
> "/Users/dpage/.virtualenvs/pgadmin4/lib/python2.7/site-
packages/flask/app.p
> y", line 1477, in full_dispatch_request
> rv = self.handle_user_exception(e)
> File
> "/Users/dpage/.virtualenvs/pgadmin4/lib/python2.7/site-
packages/flask/app.p
> y", line 1381, in handle_user_exception
> reraise(exc_type, exc_value, tb)
> File
> "/Users/dpage/.virtualenvs/pgadmin4/lib/python2.7/site-
packages/flask/app.p
> y", line 1475, in full_dispatch_request
> rv = self.dispatch_request()
> File
> "/Users/dpage/.virtualenvs/pgadmin4/lib/python2.7/site-
packages/flask/app.p
> y", line 1453, in dispatch_request
> self.raise_routing_exception(req)
> File
> "/Users/dpage/.virtualenvs/pgadmin4/lib/python2.7/site-
packages/flask/app.p
> y", line 1439, in raise_routing_exception
> raise FormDataRoutingRedirect(request)
> FormDataRoutingRedirect: A request was sent to this URL
> (http://127.0.0.1:5050/browser/server/obj/1) but a redirect was issued
> automatically by the routing system to
> "http://127.0.0.1:5050/browser/server/obj/1/". The URL was defined
> with a trailing slash so Flask will automatically redirect to the URL
> with the trailing slash if it was accessed without one. Make sure to
> directly send your PUT-request to this URL since we can't make
> browsers or HTTP clients redirect with form data reliably or without
> user interaction.
>
Did it ever work ? I was not able to test this feature before, because of
missing imports, so I just assumed it was not working in the first place.

> - Right-clicking on the tab bar and selecting any of the options to
> open new panels (Online Help, pgAdmin Website etc.) no longer
works.

Small typo from me, fixed in attached page.

>
> - When creating a new server group, they are added to the treeview
> with the name "undefined". Refreshing the page corrects the issue

> .
>
> - Deleting or renaming a server group is reflected on the treeview,
> but not saved to the database so the change is reverted on refresh.

I'll let Ashesh handle these one, probably introduced during his
refactoring of CRUD operations.

>
> - Generate Test HTML fails with:

Attachment Content-Type Size
module_api_v2.patch text/x-patch 131.1 KB

In response to

Responses

Browse pgadmin-hackers by date

  From Date Subject
Next Message Dave Page 2015-04-28 12:54:47 Re: PGAdmin 4 architecture (Was: [Patch] PGAdmin 4 JSON Handling)
Previous Message Ashesh Vashi 2015-04-28 10:53:23 Re: PGAdmin 4 architecture (Was: [Patch] PGAdmin 4 JSON Handling)