Re: [pgAdmin4][Patch]: Foreign Data Wrapper

From: Dave Page <dpage(at)pgadmin(dot)org>
To: Neel Patel <neel(dot)patel(at)enterprisedb(dot)com>
Cc: Ashesh Vashi <ashesh(dot)vashi(at)enterprisedb(dot)com>, Akshay Joshi <akshay(dot)joshi(at)enterprisedb(dot)com>, pgadmin-hackers <pgadmin-hackers(at)postgresql(dot)org>
Subject: Re: [pgAdmin4][Patch]: Foreign Data Wrapper
Date: 2016-03-17 11:41:03
Message-ID: CA+OCxow_iseEyEpD6x48XgJuZrbMEt3hoCqUEdDBuyRrR2qGVw@mail.gmail.com
Views: Raw Message | Whole Thread | Download mbox | Resend email
Thread:
Lists: pgadmin-hackers

Hi,

You missed the second from last as well, but I got that for you :-).

Thanks, committed!

On Thu, Mar 17, 2016 at 6:31 AM, Neel Patel <neel(dot)patel(at)enterprisedb(dot)com>
wrote:

> Hi Dave,
>
> Please find attached patch file with all the reported issues fixed (except
> last).
>
> Thanks,
> Neel Patel
>
> On Wed, Mar 16, 2016 at 8:44 PM, Dave Page <dpage(at)pgadmin(dot)org> wrote:
>
>> Hi
>>
>> Almost there :-)
>>
>> - The text felds on the Options grid resize when they're in Edit mode.
>> See the recent patch by Arun to prevent this in other nodes.
>>
>> - If I remove an option from the grid, then re-add it, the SQL generated
>> is backwards:
>>
>> ALTER SERVER redis_server1
>> RENAME TO redis_server;
>>
>> COMMENT ON SERVER redis_server
>> IS 'Redis Server';
>>
>> ALTER SERVER redis_server
>> OPTIONS (ADD port '6379');
>>
>> ALTER SERVER redis_server
>> OPTIONS (DROP port);
>>
>> - The dependency and dependents tabs list object types in initcap with
>> underscores instead of spaces - e.g "Foreign_data_wrapper" should be
>> "Foreign Data Wrapper"
>>
>> - I cannot select the Grantee on the security grids. See my comments a
>> few minutes ago to Ashesh/Murtuza - looks like this is a new, general bug.
>>
>> Fix those, (possibly excepting the last), and I think we're good to go.
>>
>> Thanks!
>>
>> On Tue, Mar 15, 2016 at 6:41 PM, Neel Patel <neel(dot)patel(at)enterprisedb(dot)com>
>> wrote:
>>
>>> Hi Dave,
>>>
>>> We have made changes on top of your modified patch.
>>> Please find attached patch file with all the fixes. For some of the
>>> reported issues, we are not able to reproduce it.
>>> Can you please help to reproduce the issue, if you found with latest
>>> attached patch file ?
>>>
>>> Thanks,
>>> Neel Patel
>>>
>>> On Fri, Mar 11, 2016 at 7:41 PM, Dave Page <dpage(at)pgadmin(dot)org> wrote:
>>>
>>>> OK, I'll add --binary next time. Not sure why I didn't think of that...
>>>>
>>>> Thanks.
>>>>
>>>> On Fri, Mar 11, 2016 at 2:10 PM, Ashesh Vashi <
>>>> ashesh(dot)vashi(at)enterprisedb(dot)com> wrote:
>>>>
>>>>> git diff --cached --binary
>>>>>
>>>>> --
>>>>>
>>>>> Thanks & Regards,
>>>>>
>>>>> Ashesh Vashi
>>>>> EnterpriseDB INDIA: Enterprise PostgreSQL Company
>>>>> <http://www.enterprisedb.com>
>>>>>
>>>>>
>>>>> *http://www.linkedin.com/in/asheshvashi*
>>>>> <http://www.linkedin.com/in/asheshvashi>
>>>>>
>>>>> On Fri, Mar 11, 2016 at 7:39 PM, Dave Page <dpage(at)pgadmin(dot)org> wrote:
>>>>>
>>>>>> Hmm...
>>>>>>
>>>>>> So how are you creating patches? The first one I did was by adding
>>>>>> everything I needed with "git add", then doing "git diff --cached".
>>>>>> The second one I did "git add -N" for the new files, then did a
>>>>>> regular "git diff".
>>>>>>
>>>>>> On Fri, Mar 11, 2016 at 1:33 PM, Neel Patel <
>>>>>> neel(dot)patel(at)enterprisedb(dot)com> wrote:
>>>>>> > Hi Dave,
>>>>>> >
>>>>>> > New patch file gives another kind of error. Attached is the error
>>>>>> log file
>>>>>> > for the reference.
>>>>>> > Anyway, I will use first patch file by giving command "patch -p1 "
>>>>>> which is
>>>>>> > working and manually copy the image files.
>>>>>> >
>>>>>> > Thanks,
>>>>>> > Neel Patel
>>>>>> >
>>>>>> > On Fri, Mar 11, 2016 at 6:51 PM, Dave Page <dpage(at)pgadmin(dot)org>
>>>>>> wrote:
>>>>>> >>
>>>>>> >> Alternatively, try this one.
>>>>>> >>
>>>>>> >> On Fri, Mar 11, 2016 at 1:19 PM, Dave Page <dpage(at)pgadmin(dot)org>
>>>>>> wrote:
>>>>>> >> > Huh, weird. The quick fix is this:
>>>>>> >> >
>>>>>> >> > patch -p1 < ~/foreign_data_wrapper_v5-dave.patch
>>>>>> >> >
>>>>>> >> > From the top of the source tree.
>>>>>> >> >
>>>>>> >> > On Fri, Mar 11, 2016 at 12:52 PM, Neel Patel
>>>>>> >> > <neel(dot)patel(at)enterprisedb(dot)com> wrote:
>>>>>> >> >> Hi Dave,
>>>>>> >> >>
>>>>>> >> >> I am not able to apply the attached patch file. I am getting
>>>>>> the below
>>>>>> >> >> error.
>>>>>> >> >> If possible, Can you please send correct the patch file so that
>>>>>> i can
>>>>>> >> >> fix
>>>>>> >> >> the comments on top of your patch.
>>>>>> >> >>
>>>>>> >> >>
>>>>>> >> >> ######
>>>>>> >> >> (pgAdmin4_3_4)neel(at)ubuntu:~/Projects/pgAdmin4/pgadmin4$ git
>>>>>> apply
>>>>>> >> >> foreign_data_wrapper_v5-dave.patch
>>>>>> >> >> foreign_data_wrapper_v5-dave.patch:1645: trailing whitespace.
>>>>>> >> >>
>>>>>> >> >> foreign_data_wrapper_v5-dave.patch:3067: trailing whitespace.
>>>>>> >> >>
>>>>>> >> >> foreign_data_wrapper_v5-dave.patch:3350: trailing whitespace.
>>>>>> >> >>
>>>>>> >> >> foreign_data_wrapper_v5-dave.patch:3486: trailing whitespace.
>>>>>> >> >>
>>>>>> >> >> foreign_data_wrapper_v5-dave.patch:3712: trailing whitespace.
>>>>>> >> >>
>>>>>> >> >> error: cannot apply binary patch to
>>>>>> >> >>
>>>>>> >> >>
>>>>>> 'web/pgadmin/browser/server_groups/servers/databases/foreign_data_wrappers/foreign_servers/static/img/coll-foreign_server.png'
>>>>>> >> >> without full index line
>>>>>> >> >> error:
>>>>>> >> >>
>>>>>> >> >>
>>>>>> web/pgadmin/browser/server_groups/servers/databases/foreign_data_wrappers/foreign_servers/static/img/coll-foreign_server.png:
>>>>>> >> >> patch does not apply
>>>>>> >> >> error: cannot apply binary patch to
>>>>>> >> >>
>>>>>> >> >>
>>>>>> 'web/pgadmin/browser/server_groups/servers/databases/foreign_data_wrappers/foreign_servers/static/img/foreign_server.png'
>>>>>> >> >> without full index line
>>>>>> >> >> error:
>>>>>> >> >>
>>>>>> >> >>
>>>>>> web/pgadmin/browser/server_groups/servers/databases/foreign_data_wrappers/foreign_servers/static/img/foreign_server.png:
>>>>>> >> >> patch does not apply
>>>>>> >> >> error: cannot apply binary patch to
>>>>>> >> >>
>>>>>> >> >>
>>>>>> 'web/pgadmin/browser/server_groups/servers/databases/foreign_data_wrappers/foreign_servers/user_mapping/static/img/coll-user_mapping.png'
>>>>>> >> >> without full index line
>>>>>> >> >> error:
>>>>>> >> >>
>>>>>> >> >>
>>>>>> web/pgadmin/browser/server_groups/servers/databases/foreign_data_wrappers/foreign_servers/user_mapping/static/img/coll-user_mapping.png:
>>>>>> >> >> patch does not apply
>>>>>> >> >> error: cannot apply binary patch to
>>>>>> >> >>
>>>>>> >> >>
>>>>>> 'web/pgadmin/browser/server_groups/servers/databases/foreign_data_wrappers/foreign_servers/user_mapping/static/img/user_mapping.png'
>>>>>> >> >> without full index line
>>>>>> >> >> error:
>>>>>> >> >>
>>>>>> >> >>
>>>>>> web/pgadmin/browser/server_groups/servers/databases/foreign_data_wrappers/foreign_servers/user_mapping/static/img/user_mapping.png:
>>>>>> >> >> patch does not apply
>>>>>> >> >> error: cannot apply binary patch to
>>>>>> >> >>
>>>>>> >> >>
>>>>>> 'web/pgadmin/browser/server_groups/servers/databases/foreign_data_wrappers/static/img/coll-foreign_data_wrapper.png'
>>>>>> >> >> without full index line
>>>>>> >> >> error:
>>>>>> >> >>
>>>>>> >> >>
>>>>>> web/pgadmin/browser/server_groups/servers/databases/foreign_data_wrappers/static/img/coll-foreign_data_wrapper.png:
>>>>>> >> >> patch does not apply
>>>>>> >> >> error: cannot apply binary patch to
>>>>>> >> >>
>>>>>> >> >>
>>>>>> 'web/pgadmin/browser/server_groups/servers/databases/foreign_data_wrappers/static/img/foreign_data_wrapper.png'
>>>>>> >> >> without full index line
>>>>>> >> >> error:
>>>>>> >> >>
>>>>>> >> >>
>>>>>> web/pgadmin/browser/server_groups/servers/databases/foreign_data_wrappers/static/img/foreign_data_wrapper.png:
>>>>>> >> >> patch does not apply
>>>>>> >> >>
>>>>>> >> >> #############
>>>>>> >> >>
>>>>>> >> >> On Fri, Mar 11, 2016 at 5:46 PM, Dave Page <dpage(at)pgadmin(dot)org>
>>>>>> wrote:
>>>>>> >> >>>
>>>>>> >> >>> Hi
>>>>>> >> >>>
>>>>>> >> >>> On Thu, Mar 10, 2016 at 4:58 PM, Neel Patel
>>>>>> >> >>> <neel(dot)patel(at)enterprisedb(dot)com>
>>>>>> >> >>> wrote:
>>>>>> >> >>> > Hi Dave,
>>>>>> >> >>> >
>>>>>> >> >>> > Please find attached patch file with all the fixes.
>>>>>> >> >>> > Let us know for any comments.
>>>>>> >> >>>
>>>>>> >> >>> I just spent a couple of hours looking at this. I've made
>>>>>> numerous
>>>>>> >> >>> changes for consistency with other nodes (the way they display,
>>>>>> >> >>> default values etc) - patch attached. There are a few issues
>>>>>> remaining
>>>>>> >> >>> though:
>>>>>> >> >>>
>>>>>> >> >>> - Editing a foreign data wrapper doesn't work: e.g. when
>>>>>> trying to
>>>>>> >> >>> edit a comment or rename:
>>>>>> >> >>>
>>>>>> >> >>> 2016-03-11 11:37:45,198: INFO werkzeug: 127.0.0.1 - -
>>>>>> [11/Mar/2016
>>>>>> >> >>> 11:37:45] "GET
>>>>>> >> >>>
>>>>>> >> >>>
>>>>>> /browser/foreign_data_wrapper/msql/1/2/12641/16392?id=16392&description=gffgfgfxxx&_=1457696130182
>>>>>> >> >>> HTTP/1.1" 500 -
>>>>>> >> >>> Traceback (most recent call last):
>>>>>> >> >>> File
>>>>>> >> >>>
>>>>>> >> >>>
>>>>>> "/Users/dpage/.virtualenvs/pgadmin4/lib/python2.7/site-packages/flask/app.py",
>>>>>> >> >>> line 1836, in __call__
>>>>>> >> >>> return self.wsgi_app(environ, start_response)
>>>>>> >> >>> File
>>>>>> >> >>>
>>>>>> >> >>>
>>>>>> "/Users/dpage/.virtualenvs/pgadmin4/lib/python2.7/site-packages/flask/app.py",
>>>>>> >> >>> 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.py",
>>>>>> >> >>> line 1403, in handle_exception
>>>>>> >> >>> reraise(exc_type, exc_value, tb)
>>>>>> >> >>> File
>>>>>> >> >>>
>>>>>> >> >>>
>>>>>> "/Users/dpage/.virtualenvs/pgadmin4/lib/python2.7/site-packages/flask/app.py",
>>>>>> >> >>> line 1817, in wsgi_app
>>>>>> >> >>> response = self.full_dispatch_request()
>>>>>> >> >>> File
>>>>>> >> >>>
>>>>>> >> >>>
>>>>>> "/Users/dpage/.virtualenvs/pgadmin4/lib/python2.7/site-packages/flask/app.py",
>>>>>> >> >>> line 1477, in full_dispatch_request
>>>>>> >> >>> rv = self.handle_user_exception(e)
>>>>>> >> >>> File
>>>>>> >> >>>
>>>>>> >> >>>
>>>>>> "/Users/dpage/.virtualenvs/pgadmin4/lib/python2.7/site-packages/flask/app.py",
>>>>>> >> >>> line 1381, in handle_user_exception
>>>>>> >> >>> reraise(exc_type, exc_value, tb)
>>>>>> >> >>> File
>>>>>> >> >>>
>>>>>> >> >>>
>>>>>> "/Users/dpage/.virtualenvs/pgadmin4/lib/python2.7/site-packages/flask/app.py",
>>>>>> >> >>> line 1475, in full_dispatch_request
>>>>>> >> >>> rv = self.dispatch_request()
>>>>>> >> >>> File
>>>>>> >> >>>
>>>>>> >> >>>
>>>>>> "/Users/dpage/.virtualenvs/pgadmin4/lib/python2.7/site-packages/flask/app.py",
>>>>>> >> >>> line 1461, in dispatch_request
>>>>>> >> >>> return self.view_functions[rule.endpoint](**req.view_args)
>>>>>> >> >>> File
>>>>>> >> >>>
>>>>>> >> >>>
>>>>>> "/Users/dpage/.virtualenvs/pgadmin4/lib/python2.7/site-packages/flask/views.py",
>>>>>> >> >>> line 84, in view
>>>>>> >> >>> return self.dispatch_request(*args, **kwargs)
>>>>>> >> >>> File
>>>>>> "/Users/dpage/git/pgadmin4/web/pgadmin/browser/utils.py", line
>>>>>> >> >>> 233, in dispatch_request
>>>>>> >> >>> return method(*args, **kwargs)
>>>>>> >> >>> File
>>>>>> >> >>>
>>>>>> >> >>>
>>>>>> "/Users/dpage/git/pgadmin4/web/pgadmin/browser/server_groups/servers/databases/foreign_data_wrappers/__init__.py",
>>>>>> >> >>> line 225, in wrap
>>>>>> >> >>> return f(*args, **kwargs)
>>>>>> >> >>> File
>>>>>> >> >>>
>>>>>> >> >>>
>>>>>> "/Users/dpage/git/pgadmin4/web/pgadmin/browser/server_groups/servers/databases/foreign_data_wrappers/__init__.py",
>>>>>> >> >>> line 516, in msql
>>>>>> >> >>> if sql and sql.strip('\n') and sql.strip(' '):
>>>>>> >> >>> AttributeError: 'Response' object has no attribute 'strip'
>>>>>>
>>>>> Fixed.
>>>
>>>> >> >>>
>>>>>> >> >>> - If I add an Option at the same time, the rename and comment
>>>>>> SQL is
>>>>>> >> >>> included.
>>>>>> >> >>>
>>>>>> >> >>> - If I remove the option from the list, the the SQL panel
>>>>>> still shows
>>>>>> >> >>> it.
>>>>>> >> >>>
>>>>>> >> >>> - Same issue exists for Foreign Servers.
>>>>>> >> >>>
>>>>>> >> >>> - Same issue exists when *creating* User Mappings.
>>>>>>
>>>>> >> >>>
>>>>>> >> >>> - The Dependency tab for an FDW continually says: "-- Please
>>>>>> wait
>>>>>> >> >>> while we fetch the dependency information from the server."
>>>>>> >> >>>
>>>>>>
>>>>> Not able to reproduce the issue. If possible, can you please give
>>> steps if you found the issue with this patch.
>>>
>>>> >> >>> - User mappings are not listed as dependents of their foreign
>>>>>> server.
>>>>>> >> >>> The FS does list them as a dependency though.
>>>>>>
>>>>> Fixed.
>>>
>>>> >> >>>
>>>>>> >> >>> - Why the change to
>>>>>> web/pgadmin/static/css/wcDocker/Themes/pgadmin.css
>>>>>> >> >>> ?
>>>>>>
>>>>> We made the changes to make the node icon display properly in
>>> dependencies/dependent tabs but from the latest commit, it looks good
>>> compared to older version. So no need to include in the patch.
>>>
>>>> >> >>>
>>>>>> >> >>> - On the User Mappings dialogue, can we add CURRENT_USER and
>>>>>> PUBLIC as
>>>>>> >> >>> additional options to the Username dropdown?
>>>>>>
>>>>> Fixed.
>>>
>>>> >> >>>
>>>>>> >> >>> Otherwise, I think this is pretty much there. Thanks.
>>>>>> >> >>>
>>>>>> >> >>> --
>>>>>> >> >>> Dave Page
>>>>>> >> >>> Blog: http://pgsnake.blogspot.com
>>>>>> >> >>> Twitter: @pgsnake
>>>>>> >> >>>
>>>>>> >> >>> EnterpriseDB UK: http://www.enterprisedb.com
>>>>>> >> >>> The Enterprise PostgreSQL Company
>>>>>> >> >>
>>>>>> >> >>
>>>>>> >> >
>>>>>> >> >
>>>>>> >> >
>>>>>> >> > --
>>>>>> >> > Dave Page
>>>>>> >> > Blog: http://pgsnake.blogspot.com
>>>>>> >> > Twitter: @pgsnake
>>>>>> >> >
>>>>>> >> > EnterpriseDB UK: http://www.enterprisedb.com
>>>>>> >> > The Enterprise PostgreSQL Company
>>>>>> >>
>>>>>> >>
>>>>>> >>
>>>>>> >> --
>>>>>> >> Dave Page
>>>>>> >> Blog: http://pgsnake.blogspot.com
>>>>>> >> Twitter: @pgsnake
>>>>>> >>
>>>>>> >> EnterpriseDB UK: http://www.enterprisedb.com
>>>>>> >> The Enterprise PostgreSQL Company
>>>>>> >
>>>>>> >
>>>>>>
>>>>>>
>>>>>>
>>>>>> --
>>>>>> Dave Page
>>>>>> Blog: http://pgsnake.blogspot.com
>>>>>> Twitter: @pgsnake
>>>>>>
>>>>>> EnterpriseDB UK: http://www.enterprisedb.com
>>>>>> The Enterprise PostgreSQL Company
>>>>>>
>>>>>>
>>>>>> --
>>>>>> Sent via pgadmin-hackers mailing list (pgadmin-hackers(at)postgresql(dot)org
>>>>>> )
>>>>>> To make changes to your subscription:
>>>>>> http://www.postgresql.org/mailpref/pgadmin-hackers
>>>>>>
>>>>>
>>>>>
>>>>
>>>>
>>>> --
>>>> Dave Page
>>>> Blog: http://pgsnake.blogspot.com
>>>> Twitter: @pgsnake
>>>>
>>>> EnterpriseDB UK: http://www.enterprisedb.com
>>>> The Enterprise PostgreSQL Company
>>>>
>>>
>>>
>>
>>
>> --
>> Dave Page
>> Blog: http://pgsnake.blogspot.com
>> Twitter: @pgsnake
>>
>> EnterpriseDB UK: http://www.enterprisedb.com
>> The Enterprise PostgreSQL Company
>>
>
>

--
Dave Page
Blog: http://pgsnake.blogspot.com
Twitter: @pgsnake

EnterpriseDB UK: http://www.enterprisedb.com
The Enterprise PostgreSQL Company

In response to

Responses

Browse pgadmin-hackers by date

  From Date Subject
Next Message Dave Page 2016-03-17 11:49:34 pgAdmin 4 commit: Support "show system objects" in casts.
Previous Message Dave Page 2016-03-17 11:39:58 pgAdmin 4 commit: Add support for Foreign data wrappers, servers and us