Re: PGAdmin 4 architecture (Was: [Patch] PGAdmin 4 JSON Handling) - Mailing list pgadmin-hackers

Le mardi 28 avril 2015 10:00:08 Dave Page a écrit :
> On Thu, Apr 23, 2015 at 12:42 PM, Ronan Dunklau
>
> <ronan.dunklau@dalibo.com> wrote:
> > 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.
>
> 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.

>
> (pgadmin4)piranha:web dpage$ python pgAdmin4.py
> 2015-04-28 09:57:14,966: INFO pgadmin:
> ############################################################################
> #### 2015-04-28 09:57:14,966: INFO pgadmin: Starting pgAdmin 4 v1.0.0-dev...
> 2015-04-28 09:57:14,966: INFO pgadmin:
> ############################################################################
> #### 2015-04-28 09:57:14,966: DEBUG pgadmin: Python syspath:
> ['/Users/dpage/git/pgadmin4/web',
> '/Users/dpage/.virtualenvs/pgadmin4/lib/python2.7/site-packages/setuptools-0
> .6c11-py2.7.egg',
> '/Users/dpage/.virtualenvs/pgadmin4/lib/python2.7/site-packages/pip-6.0.8-p
> y2.7.egg',
> '/Users/dpage/.virtualenvs/pgadmin4/lib/python2.7/site-packages/pytz-2014.1
> 0-py2.7.egg', '/Users/dpage/.virtualenvs/pgadmin4/lib/python27.zip',
> '/Users/dpage/.virtualenvs/pgadmin4/lib/python2.7',
> '/Users/dpage/.virtualenvs/pgadmin4/lib/python2.7/plat-darwin',
> '/Users/dpage/.virtualenvs/pgadmin4/lib/python2.7/plat-mac',
> '/Users/dpage/.virtualenvs/pgadmin4/lib/python2.7/plat-mac/lib-scriptpackage
> s', '/Users/dpage/.virtualenvs/pgadmin4/Extras/lib/python',
> '/Users/dpage/.virtualenvs/pgadmin4/lib/python2.7/lib-tk',
> '/Users/dpage/.virtualenvs/pgadmin4/lib/python2.7/lib-old',
> '/Users/dpage/.virtualenvs/pgadmin4/lib/python2.7/lib-dynload',
> '/System/Library/Frameworks/Python.framework/Versions/2.7/lib/python2.7',
> '/System/Library/Frameworks/Python.framework/Versions/2.7/lib/python2.7/plat
> -darwin',
> '/System/Library/Frameworks/Python.framework/Versions/2.7/lib/python2.7/lib
> -tk',
> '/System/Library/Frameworks/Python.framework/Versions/2.7/lib/python2.7/pla
> t-mac',
> '/System/Library/Frameworks/Python.framework/Versions/2.7/lib/python2.7/pla
> t-mac/lib-scriptpackages',
> '/Users/dpage/.virtualenvs/pgadmin4/lib/python2.7/site-packages']
> 2015-04-28 09:57:14,967: DEBUG pgadmin: Available translations:
> [Locale('fr')] 2015-04-28 09:57:14,976: INFO pgadmin: Examining potential
> module: pgadmin.about 2015-04-28 09:57:14,977: INFO pgadmin: Registering
> blueprint module: <pgadmin.about.AboutModule object at 0x10b1e5410>
> 2015-04-28 09:57:14,977: INFO pgadmin: Examining potential module:
> pgadmin.about.hooks
> 2015-04-28 09:57:14,977: INFO pgadmin: Examining potential module:
> pgadmin.about.views
> 2015-04-28 09:57:14,978: INFO pgadmin: Examining potential module:
> pgadmin.browser
> 2015-04-28 09:57:14,979: INFO pgadmin: Registering blueprint module:
> <pgadmin.browser.BrowserModule object at 0x10b23e5d0>
> 2015-04-28 09:57:14,979: INFO pgadmin: Examining potential module:
> pgadmin.browser.hooks
> Traceback (most recent call last):
>   File "pgAdmin4.py", line 51, in <module>
>     app = create_app()
>   File "/Users/dpage/git/pgadmin4/web/pgadmin/__init__.py", line 160,
> in create_app
>     app.register_blueprint(module)
>   File
> "/Users/dpage/.virtualenvs/pgadmin4/lib/python2.7/site-packages/flask/app.p
> y", line 62, in wrapper_func
>     return f(self, *args, **kwargs)
>   File
> "/Users/dpage/.virtualenvs/pgadmin4/lib/python2.7/site-packages/flask/app.p
> y", line 889, in register_blueprint
>     blueprint.register(self, options, first_registration)
>   File "/Users/dpage/git/pgadmin4/web/pgadmin/utils/__init__.py", line
> 29, in register
>     self.submodules = list(app.find_submodules(self.import_name))
>   File "/Users/dpage/git/pgadmin4/web/pgadmin/__init__.py", line 40,
> in find_submodules
>     module = import_module(module_name)
>   File
> "/System/Library/Frameworks/Python.framework/Versions/2.7/lib/python2.7/imp
> ortlib/__init__.py", line 37, in import_module
>     __import__(name)
>   File "/Users/dpage/git/pgadmin4/web/pgadmin/browser/hooks.py", line
> 15, in <module>
> 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

When developing, it can be useful to set the env variable
PYTHONDONTWRITEBYTECODE to prevent the interpreter from generating those
files.


>
> > 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.

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