From: | Neel Patel <neel(dot)patel(at)enterprisedb(dot)com> |
---|---|
To: | Dave Page <dpage(at)pgadmin(dot)org> |
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 12:14:47 |
Message-ID: | CACCA4P2Yy+ArJ0HrhA6EMgJQx2kqK_U1aBC7LSjecLrewnVfWQ@mail.gmail.com |
Views: | Raw Message | Whole Thread | Download mbox | Resend email |
Thread: | |
Lists: | pgadmin-hackers |
Thank you dave for the change.
In Version 6 patch file, I made the same change but don't know why were you
not be able to see the object type with space ( may be depends.js was not
updated ). After your comment i thought you want with underscore so i again
change the logic. :)
Thanks,
Neel Patel
On Thu, Mar 17, 2016 at 5:11 PM, Dave Page <dpage(at)pgadmin(dot)org> wrote:
> 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
>
From | Date | Subject | |
---|---|---|---|
Next Message | Dave Page | 2016-03-17 13:11:18 | Re: Subnode grid row close issue [pgAdmin4] |
Previous Message | Dave Page | 2016-03-17 11:49:50 | Re: patch for cast module |