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

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.patel@enterprisedb.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@pgadmin.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.vashi@enterprisedb.com> wrote:

git diff --cached --binary

--

Thanks & Regards,

Ashesh Vashi
EnterpriseDB INDIA: Enterprise PostgreSQL Company

http://www.linkedin.com/in/asheshvashi

On Fri, Mar 11, 2016 at 7:39 PM, Dave Page <dpage@pgadmin.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.patel@enterprisedb.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@pgadmin.org> wrote:
>>
>> Alternatively, try this one.
>>
>> On Fri, Mar 11, 2016 at 1:19 PM, Dave Page <dpage@pgadmin.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.patel@enterprisedb.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@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@pgadmin.org> wrote:
>> >>>
>> >>> Hi
>> >>>
>> >>> On Thu, Mar 10, 2016 at 4:58 PM, Neel Patel
>> >>> <neel.patel@enterprisedb.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@postgresql.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