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

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
>