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

Поиск
Список
Период
Сортировка
От Neel Patel
Тема Re: [pgAdmin4][Patch]: Foreign Data Wrapper
Дата
Msg-id CACCA4P1dOTi1MD0EaM84EFgN1Rh2Y90BF-T6HrhbwhbmxXe5gw@mail.gmail.com
обсуждение исходный текст
Ответ на Re: [pgAdmin4][Patch]: Foreign Data Wrapper  (Dave Page <dpage@pgadmin.org>)
Ответы Re: [pgAdmin4][Patch]: Foreign Data Wrapper  (Dave Page <dpage@pgadmin.org>)
Список pgadmin-hackers
Hi Dave,

Thank you for the reviewing the patch.
Please find inline comments and attached updated patch file after fixing all the comments.

Do let us know in case of any comments.

Thanks,
Neel Patel

On Fri, Mar 4, 2016 at 10:19 PM, Dave Page <dpage@pgadmin.org> wrote:
Hi

On Tue, Feb 23, 2016 at 5:24 PM, Neel Patel <neel.patel@enterprisedb.com> wrote:
Hi,

Please find attached patch file for the following three nodes.
  • Foreign Data Wrappers
  • Foreign Servers
  • User Mapping
With this patch, we have implemented "Dependencies", "Dependent" tab and proper comments has been added.

Do review it and let us know for any comments.

This seems to be nearly ready now. Some feedback below - note that a couple of the issues may be caused by infrastructure code, in which case please do fix them, but feel free to put them in a different patch:

- When adding a User Mapping, you cannot specify an empty option, e.g. a blank password.
 
Fixed. User will not be able to save with blank password. "Save" button will be disabled and error message will be displayed.
 

- When granting USAGE to PUBLIC on a foreign server, the WITH GRANT OPTION is set, despite not being selected. e.g. adding "U" permissions for "public" results in:

GRANT ALL ON FOREIGN SERVER redis_server TO public;
GRANT ALL ON FOREIGN SERVER redis_server TO public WITH GRANT OPTION;
 
Fixed.
 

- Why all the extra blank lines in this SQL? I would expect to see only 2, between the ALTER/COMMENT/GRANT sections.

====
ALTER SERVER redis_server
    VERSION 'Fooo';

COMMENT ON SERVER redis_server
    IS 'Redis Server x';






GRANT ALL ON FOREIGN SERVER redis_server TO public;
GRANT ALL ON FOREIGN SERVER redis_server TO public WITH GRANT OPTION;


====
 
Fixed.
 

- Error messages from the server are not displayed properly - e.g:

invalid option "server"
HINT:  Valid options in this context are: <none>

Is displayed as:

invalid option "server" HINT: Valid options in this context are:

(Notice the lack of "<none>")

Here we are displaying the generic error message text received from database server. We can not add <none> to message text but "HINT" will be displayed in new line.

e.g. If you execute below command in PG 9.5 through "plpgsql" then database server will give the error message without valid context.

CREATE FOREIGN DATA WRAPPER test_fdw VALIDATOR postgresql_fdw_validator OPTIONS (server '123');

Error message will be as below from database server.

ERROR:  invalid option "server"
HINT:  Valid options in this context are: 


- A foreign server object is not listed as being dependent upon the FDW it's defined as using (but, pgAdmin 3 gets this wrong too). 

Fixed.
 

- The "Options" tabs have a hint message of "Please enter some value!", whether the focus is in the Option or Value field. Please fix to display:

Please enter an option name.

or

Please enter a value.

Based on the current focus. Note also that there should not be an exclamation mark.

Fixed.
 

Thanks!

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

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

Вложения

В списке pgadmin-hackers по дате отправления:

Предыдущее
От: Harshal Dhumal
Дата:
Сообщение: Re: Patch sequence node [pgadmin4]
Следующее
От: Surinder Kumar
Дата:
Сообщение: Re: [pgAdmin4] [Patch]: Grant Wizard