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

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
>