[pgadmin4][patch] GreenPlum error while checking roles SQL

[Joao De Almeida Pereira]

Hi Hackers,

Attached you can find a patch that will correct issue #3578

The SQL command used to retrieve the SQL for the Role does not work,
because it as a piece of it that is intended for Postgres > 9.0.

Thanks
Joao
diff --git a/web/pgadmin/browser/server_groups/servers/roles/templates/role/sql/post8_4/sql.sql b/web/pgadmin/browser/server_groups/servers/roles/templates/role/sql/post8_4/sql.sql
index 642009e0..d3cb081b 100644
--- a/web/pgadmin/browser/server_groups/servers/roles/templates/role/sql/post8_4/sql.sql
+++ b/web/pgadmin/browser/server_groups/servers/roles/templates/role/sql/post8_4/sql.sql
@@ -59,35 +59,6 @@ FROM
 		oid=%(rid)s::OID
 	) r
 ) a) b)
--- PostgreSQL >= 9.0
-UNION ALL
-(SELECT
-	array_to_string(array_agg(sql), E'\n') AS sql
-FROM
-	(SELECT
-		'ALTER ROLE ' || pg_catalog.quote_ident(pg_get_userbyid(%(rid)s::OID)) ||
-		' SET ' || param|| ' TO ' ||
-		CASE
-		WHEN param IN ('search_path', 'temp_tablespaces') THEN value
-		ELSE quote_literal(value)
-		END || ';' AS sql
-	FROM
-		(SELECT
-			datname, split_part(rolconfig, '=', 1) AS param, replace(rolconfig, split_part(rolconfig, '=', 1) || '=', '') AS value
-		FROM
-			(SELECT
-d.datname, unnest(c.setconfig) AS rolconfig
-			FROM
-(SELECT *
-FROM
-	pg_catalog.pg_db_role_setting dr
-WHERE
-	dr.setrole=%(rid)s::OID AND dr.setdatabase!=0) c
-LEFT JOIN pg_catalog.pg_database d ON (d.oid = c.setdatabase)
-			) a
-		) b
-	) d
-)
 UNION ALL
 (SELECT
 	'COMMENT ON ROLE ' || pg_catalog.quote_ident(pg_get_userbyid(%(rid)s::OID)) || ' IS ' ||  pg_catalog.quote_literal(description) || ';' AS sql

[pgadmin4][patch] Correct issue on external tables

[Joao De Almeida Pereira]

Hi Hackers,

You can find attached a patch that corrects the RM #3431.

When trying to retrieve the DDL of an external table and exceptions was
being raised.

Thanks
Joao
diff --git a/web/pgadmin/browser/server_groups/servers/databases/external_tables/templates/sql/gpdb_5.0_plus/create.sql b/web/pgadmin/browser/server_groups/servers/databases/external_tables/templates/sql/gpdb_5.0_plus/create.sql
index a7f64a5f..25ebf96c 100644
--- a/web/pgadmin/browser/server_groups/servers/databases/external_tables/templates/sql/gpdb_5.0_plus/create.sql
+++ b/web/pgadmin/browser/server_groups/servers/databases/external_tables/templates/sql/gpdb_5.0_plus/create.sql
@@ -48,7 +48,7 @@ OPTIONS (
 )
 {% endif %}
 ENCODING '{{ table.pgEncodingToChar }}'
-{% if table.rejectLimit and table.rejectLimit|length > 0 %}
+{% if table.rejectLimit and table.rejectLimit > 0 %}
 {%   if table.errorTableName and table.errorTableName|length > 0 %}
 LOG ERRORS {% endif %}SEGMENT REJECT LIMIT {{ table.rejectLimit }} {{ rejectionLimit }}
 {% endif %}

Re: [pgadmin4][Patch]: Support for create multiple test classes in one test file

[Joao De Almeida Pereira]

Hello Hackers,
Instead of doing this change and include more test harnessing to the setup
we have, do you think we can try to push to get pytest into the code base
and all these features will come by default?

Thanks
Joao
​

On Tue, Jun 12, 2018 at 9:30 AM Akshay Joshi 
wrote:

> Hi Hackers,
>
> Attached is the patch to create multiple test classes in one test file.
>
> For example: I have one test file *test_feature.py *where I wrote
> multiple test classes
> class TestX(BaseTestGenerator):
>   class TestY(BaseTestGenerator):
>   class TestZ(BaseTestGenerator):
>
> So with current implementation it will run the test cases for *TestZ*
> class.
>
> --
> *Akshay Joshi*
>
> *Sr. Software Architect *
>
>
>
> *Phone: +91 20-3058-9517Mobile: +91 976-788-8246*
>

[pgadmin4][patch][GreenPlum] Display SQL for tables takes 15 minutes to execute

[Joao De Almeida Pereira]

Hi Hackers,

Attached you can find the diff file that corrects RM 3415

​
Thanks
Joao


RM_3415.diff
Description: Binary data

Re: [pgAdmin4][RM#3289] Can't query SQL_ASCII database.

[Joao De Almeida Pereira]

Hello Aditya,

>
>
> There is no change related to notifications in this patch.
> The below code is minor fix related to connection status of sql editor.
> Can you please share the code snippet if it is not the below.
>
> -# Check for the asynchronous notifies statements.
> -conn.check_notifies(True)
> -notifies = conn.get_notifies()
> +if status is not None:
> +# Check for the asynchronous notifies statements.
> +conn.check_notifies(True)
> +notifies = conn.get_notifies()
>
>
This is a minor fix, but is it related to querying SQL_ASCII database?

Thanks
Victoria && Joao

Re: [pgAdmin4][RM#3289] Can't query SQL_ASCII database.

[Joao De Almeida Pereira]

Hi Aditya,
Looks like there are changes in this patch that are related to
notifications, these should go into a separate commit as they are not
related to encoding.
Also the tests are failing in our pipeline:
https://gpdb-dev.bosh.pivotalci.info/teams/pgadmin/pipelines/pgadmin-patch/jobs/run-tests/builds/110

Thanks
Victoria && Joao

On Mon, Jun 4, 2018 at 5:55 AM Aditya Toshniwal <
aditya.toshni...@enterprisedb.com> wrote:

> Hi Hackers,
>
> Please ignore previous patch. Fixed some linter issues. PFA updated patch.
>
> Thanks and Regards,
> Aditya Toshniwal
> Software Engineer | EnterpriseDB Software Solutions | Pune
> "Don't Complain about Heat, Plant a tree"
>
> On Mon, Jun 4, 2018 at 10:53 AM, Aditya Toshniwal <
> aditya.toshni...@enterprisedb.com> wrote:
>
>> Hi Hackers,
>>
>> PFA the updated patch on this. I have tried to add test cases to check
>> for different encoding database. In the test run, it will create a
>> database, fire a query for a string, check if we get the output and drops
>> the database.
>> Kindly review.
>>
>> Thanks and Regards,
>> Aditya Toshniwal
>> Software Engineer | EnterpriseDB Software Solutions | Pune
>> "Don't Complain about Heat, Plant a tree"
>>
>> On Thu, May 31, 2018 at 6:42 PM, Dave Page  wrote:
>>
>>> Hi
>>>
>>> On Thu, May 31, 2018 at 1:20 AM, Aditya Toshniwal <
>>> aditya.toshni...@enterprisedb.com> wrote:
>>>
>>>> Hi Victoria/Joao,
>>>>
>>>> On Thu, May 31, 2018 at 2:06 AM, Joao De Almeida Pereira <
>>>> jdealmeidapere...@pivotal.io> wrote:
>>>>
>>>>> Hello Aditya,
>>>>>
>>>>> It looks ok and it passes CI.
>>>>>
>>>>> We have some recommendations:
>>>>> - These look like 2 different changes so they should be in separated
>>>>> commits
>>>>>
>>>>
>>>> If you are talking of set client_encoding, then its not a bug. Its a
>>>> choice given to Postgres DB user to change the encoding of the characters.
>>>> Postgres will translate characters from Server Encoding to Client Encoding,
>>>> and will throw error like mentioned previously. This link will help better
>>>> - https://www.postgresql.org/docs/10/static/multibyte.html
>>>> The actual bug was, even after changing the client encoding to
>>>> SQL_ASCII, pgAdmin4 was not able to show the output as it was failing in
>>>> encoding by psycopg2. The patch is for resolving that.
>>>>
>>>>
>>>>> - Do we have test coverage for the bug that you are talking about? If
>>>>> not we should, to ensure this problem does not happen again in a future
>>>>> change.
>>>>>
>>>>
>>>> It is not possible adding test cases for encoding related stuff, as
>>>> Postgres support a lot many different types of encoding and conversions
>>>> (refer above link)
>>>>
>>>
>>> I was going to ask the same thing. Per
>>> https://www.postgresql.org/docs/10/static/multibyte.html#id-1.6.10.5.7,
>>> every characterset except SQL_ASCII can be converted to UTF-8, so we only
>>> need to test that UTF-8 and some other charactersets besides SQL_ASCII
>>> work, and then separately that SQL_ASCII with characters known not to be in
>>> UTF-8 work.
>>>
>>>
>>>>
>>>>> Thanks
>>>>> Victoria && Joao
>>>>>
>>>>> On Wed, May 30, 2018 at 3:06 AM Aditya Toshniwal <
>>>>> aditya.toshni...@enterprisedb.com> wrote:
>>>>>
>>>>>> Hi Hackers,
>>>>>>
>>>>>> PFA updated patch after all the permutations, combinations for
>>>>>> encoding for SQL_ASCII database.  Also fixed a small glitch for sql
>>>>>> editor connection status check.
>>>>>>
>>>>>> Please note, ERROR: invalid byte sequence for encoding "UTF8": 0xe9
>>>>>> 0x73 is a Postgres DB error and not pgAdmin4 error.
>>>>>>
>>>>>> 
>>>>>>
>>>>>> You need to change client_encoding to the appropriate. After changing
>>>>>> client_encoding using command - set client_encoding='XYZ', it will give 
>>>>>> not
>>>>>> give error.
>>>>>>
>>>>>> 
>>>>>>
>>>>&

Re: [pgadmin4][Patch]: Test cases for the backup module

[Joao De Almeida Pereira]

>> khushboo.va...@enterprisedb.com> wrote:
>>>>
>>>>> Hi,
>>>>>
>>>>> Please find the attached updated patch with the fixes.
>>>>> The test cases were only failing on MAC not on Linux.
>>>>>
>>>>> Thanks,
>>>>> Khushboo
>>>>>
>>>>> On Wed, May 30, 2018 at 10:13 AM, Khushboo Vashi <
>>>>> khushboo.va...@enterprisedb.com> wrote:
>>>>>
>>>>>>
>>>>>>
>>>>>> On Wed, May 30, 2018 at 1:05 AM, Dave Page  wrote:
>>>>>>
>>>>>>> Hi
>>>>>>>
>>>>>>> On Mon, May 28, 2018 at 8:09 AM, Khushboo Vashi <
>>>>>>> khushboo.va...@enterprisedb.com> wrote:
>>>>>>>
>>>>>>>> Hi,
>>>>>>>>
>>>>>>>> please find the attached updated patch for the test cases of
>>>>>>>> Backup, Restore and Maintenance modules which includes:
>>>>>>>>
>>>>>>>> 1. Unit test cases
>>>>>>>> 2. End to end regression test cases
>>>>>>>> 3. Feature test cases
>>>>>>>>
>>>>>>>
>>>>>>> Thanks. I've yet to be able to run the feature tests successfully.
>>>>>>> Here's what I've found so far:
>>>>>>>
>>>>>>> 1) DEFAULT_BINARY_PATHS should be default_binary_paths in the JSON
>>>>>>> config file.
>>>>>>>
>>>>>>> Will do.
>>>>>>
>>>>>>> 2) I've hit screensize related issues:
>>>>>>>
>>>>>>>
>>>>>>> ==
>>>>>>>
>>>>>>> ERROR: runTest
>>>>>>> (pgadmin.feature_tests.pg_utilities_maintenance_test.PGUtilitiesMaintenanceFeatureTest)
>>>>>>>
>>>>>>> Test for PG maintenance: database
>>>>>>>
>>>>>>>
>>>>>>> --
>>>>>>>
>>>>>>> Traceback (most recent call last):
>>>>>>>
>>>>>>>   File
>>>>>>> "/Users/dpage/git/pgadmin4/web/pgadmin/feature_tests/pg_utilities_maintenance_test.py",
>>>>>>> line 56, in runTest
>>>>>>>
>>>>>>> self._open_maintenance_dialogue()
>>>>>>>
>>>>>>>   File
>>>>>>> "/Users/dpage/git/pgadmin4/web/pgadmin/feature_tests/pg_utilities_maintenance_test.py",
>>>>>>> line 75, in _open_maintenance_dialogue
>>>>>>>
>>>>>>> "*[.='" + self.table_name + "']/../*[@class='aciTreeItem'"
>>>>>>>
>>>>>>>   File
>>>>>>> "/Users/dpage/.virtualenvs/pgadmin4/lib/python2.7/site-packages/selenium/webdriver/remote/webelement.py",
>>>>>>> line 80, in click
>>>>>>>
>>>>>>> self._execute(Command.CLICK_ELEMENT)
>>>>>>>
>>>>>>>   File
>>>>>>> "/Users/dpage/.virtualenvs/pgadmin4/lib/python2.7/site-packages/selenium/webdriver/remote/webelement.py",
>>>>>>> line 628, in _execute
>>>>>>>
>>>>>>> return self._parent.execute(command, params)
>>>>>>>
>>>>>>>   File
>>>>>>> "/Users/dpage/.virtualenvs/pgadmin4/lib/python2.7/site-packages/selenium/webdriver/remote/webdriver.py",
>>>>>>> line 312, in execute
>>>>>>>
>>>>>>> self.error_handler.check_response(response)
>>>>>>>
>>>>>>>   File
>>>>>>> "/Users/dpage/.virtualenvs/pgadmin4/lib/python2.7/site-packages/selenium/webdriver/remote/errorhandler.py",
>>>>>>> line 242, in check_response
>>>>>>>
>>>>>>> raise exception_class(message, screen, stacktrace)
>>>>>>>
>>>>>>> WebDriverException: Message: unknown error: Element >>>>>> class="aciTreeItem">... is not clickable at point (223, 604). 
>>>>>>> Other
>>>>>>> 

Re: [pgAdmin4][Patch] Feature #3204 Notify/Listen not working in version 2.1

[Joao De Almeida Pereira]

Hello Akshay,

Thanks for taking a look. We did the following changes over your patch:

   - Changed the XPATH to CSS_SELECTOR, please look in
pgadmin/feature_tests/query_tool_tests.py:660
   and 667
   - Changed the other places in _query_tool_notify_statements to do not
   use xpath.
   - Moved the skip that previously was around the entire function to
   surround only the pg_notify call

Thanks
Victoria && Joao
​

On Wed, May 30, 2018 at 1:52 AM Akshay Joshi 
wrote:

> Hi Joao
>
> On Tue, May 29, 2018 at 9:10 PM, Joao De Almeida Pereira <
> jdealmeidapere...@pivotal.io> wrote:
>
>> Hello Akshay,
>>
>> The code looks good, we do have some minor notes for next time:
>>
>> . To make the code easier to understand we should decide on 1 term and
>> not have notifies and notifications to reference the same thing.
>>
>  Fixed and used 'notifies' where it reference the same, but still at
> some places 'notifications' is used because the new tab represents the
> 'Notifications'.
>
>
>> . It would be nice if we tried not to use XPATH to get the HTML
>> components that we are testing on. Maybe try to use CSS classes. Selenium
>> also has a driver.select_by_class_name which would work well in this
>> case.
>>
>  Tried to use both CSS_SELECTOR and CLASS_NAME, but it doesn't
> support string comparison(contains method). I have googled for this and
> most of the sites suggested to use XPATH where we can use contains()
> method.
>
>
>> The error that we are seeing in CI is related to pg_notify. This function
>> was introduced in 9.0 and Greenplum is still not there yet. In order to
>> solve this need to skip that part of the tests. There is an example in that
>> same file using the function _test_explain_plan_feature to skip a tests
>> depending on the Version.
>>
> OK. I have skip that part of the tests as per your suggestion. I have
> rename the method "_test_explain_plan_feature" to
> "_supported_server_version" which is more meaningful then the previous one.
>
>Attached is the modified patch. Please review it.
>
>
>> Thanks
>> Anthony && Joao
>> ​
>>
>> On Mon, May 28, 2018 at 1:34 AM Akshay Joshi <
>> akshay.jo...@enterprisedb.com> wrote:
>>
>>> Hi Hakers,
>>>
>>> On my last patch feature tests were failed for GreenPlum5 database only
>>> not for PostgreSQL. I have verified that on
>>> https://gpdb-dev.bosh.pivotalci.info/teams/pgadmin/pipelines/pgadmin-patch/jobs/run-tests/builds/93
>>> .
>>>
>>> Can someone from Pivotal team help me, as I don't have GreenPlum
>>> database and don't know why it is failing.
>>>
>>> On Sat, May 26, 2018 at 5:47 PM, Akshay Joshi <
>>> akshay.jo...@enterprisedb.com> wrote:
>>>
>>>> Hi Hackers,
>>>>
>>>> As per suggestion by Dave and discussion with in the team, I have
>>>> modified the logic again. Following are the modifications:
>>>>
>>>>- Instead of waiting for another query to execute on the session
>>>>where 'LISTEN' command has been executed, we fetched the notify 
>>>> messages in
>>>>the connection status polling logic. Doing this user will get the notify
>>>>messages asap.
>>>>- Instead of showing all the notifications in single alertify dialog,
>>>>we call *alertify.info <http://alertify.info>('') *for
>>>>individual notifications.
>>>>- Created new tab 'Notifications' in Query Tool where all the
>>>>notify messages will be recorded with the timestamp and payload.
>>>>
>>>> Please review the latest patch.
>>>>
>>>> On Tue, May 22, 2018 at 2:43 PM, Dave Page  wrote:
>>>>
>>>>>
>>>>>
>>>>> On Tue, May 22, 2018 at 10:01 AM, Akshay Joshi <
>>>>> akshay.jo...@enterprisedb.com> wrote:
>>>>>
>>>>>> Hi Dave
>>>>>>
>>>>>> On Tue, May 22, 2018 at 2:02 PM, Dave Page  wrote:
>>>>>>
>>>>>>>
>>>>>>>
>>>>>>> On Tue, May 22, 2018 at 9:13 AM, Dave Page 
>>>>>>> wrote:
>>>>>>>
>>>>>>>> Hi
>>>>>>>>
>>>>>>>> On Tue, May 22, 2018 at 7:07 AM, Akshay Joshi <
>>>>>>>> akshay.jo...@enterprisedb.com> wrote:
>>>>>>>>
>>>>>

Re: pgAdmin 4 commit: Disable emojis in Yarn output.

[Joao De Almeida Pereira]

Hello Dave,
When running yarn linter after this commit we get the following:

yarn run v1.7.0
$ yarn eslint --emoji false --no-eslintrc -c .eslintrc.js --ext .js --ext .jsx .
$ /Users/pivotal/workspace/pgadmin4/web/node_modules/.bin/eslint
--emoji false --no-eslintrc -c .eslintrc.js --ext .js --ext .jsx .
Invalid option '--emoji' - perhaps you meant '--env'?
error Command failed with exit code 1.
info Visit https://yarnpkg.com/en/docs/cli/run for documentation about
this command.
error Command failed with exit code 1.
info Visit https://yarnpkg.com/en/docs/cli/run for documentation about
this command.

Thanks
Victoria && Joao
​

On Wed, May 30, 2018 at 4:42 PM Dave Page  wrote:

> Disable emojis in Yarn output.
>
> Branch
> --
> master
>
> Details
> ---
>
> https://git.postgresql.org/gitweb?p=pgadmin4.git;a=commitdiff;h=b2d4c6ef9d7715c62310bd05dace97f7523a527d
>
> Modified Files
> --
> pkg/docker/Dockerfile |  4 ++--
> web/package.json  | 20 ++--
> 2 files changed, 12 insertions(+), 12 deletions(-)
>
>

Re: [pgAdmin4][Patch#3389] To prevent unwanted model changes in Server dialog

[Joao De Almeida Pereira]

Hi Murtuza,

We are having a hard time understanding the logic on the change.

If tunnel_authentication is not on the model and the tunnel_identify_file
is present, we change the tunnel_identify_file in the model to null? Is
this what you mean?

Thanks
Victoria && Joao
​

On Wed, May 30, 2018 at 9:27 AM Murtuza Zabuawala <
murtuza.zabuaw...@enterprisedb.com> wrote:

> Hi,
>
> PFA minor patch to fix the issue when you change any field in server
> dialog 'tunnel_identity_file' model value get included unnecessarily in the
> update request.
>
> eg: Change the server name and click on Save button, Check the request
> payload in network console.
>
> --
> Regards,
> Murtuza Zabuawala
> EnterpriseDB: http://www.enterprisedb.com
> The Enterprise PostgreSQL Company
>
>

Re: [PATCH] Support for the source-map in Karma test framework

[Joao De Almeida Pereira]

Hello Ashesh

The change looks great. This will help the community a lot when we are
TDDing new features.

Please do not forget NOT to commit the second patch.

Thanks
Victoria && Joao

On Wed, May 30, 2018 at 3:51 AM Ashesh Vashi 
wrote:

> Hi Team,
>
> Debugging the failed jasmine test-cases is very difficult with the current
> implementation.
> The stack-traces does not give the correct information as It does not
> have source-map support for the original code.
> And, that makes it very difficult to debug the issue during development.
>
> Please attached a patch for adding the support for the source-map in the
> karma test framework.
>
> I have also attached another patch, which will definitely fail some of the
> test-cases to show the stack-traces of the failed cases.
>
> Please find the output of 'yarn run test:karma-once' before applying the
> proposed patch as below:
>
> *...*
>
> *HeadlessChrome 0.0.0 (Mac OS X 10.11.6) tree tests TreeNode #hasParent
> parent exists returns true FAILED*
> * Expected true to be false. at UserContext.
> (regression/javascript/tree/tree_spec.js:780:40)HeadlessChrome 0.0.0 (Mac
> OS X 10.11.6): Executed 534 of 586 (1 FAILED) (0 secs / 10.361
> secs)HeadlessChrome 0.0.0 (Mac OS X 10.11.6) tree tests TreeNode #hasParent
> parent exists returns true FAILED Expected true to be false.HeadlessChrome
> 0.0.0 (Mac OS X 10.11.6) tree tests TreeNode #reload reloads the node and
> its children FAILED TypeError: Cannot read property 'fakeFail' of undefined
> at TreeFake.addNewNode (regression/javascript/tree/tree_spec.js:237:22)
> at TreeFake.addNewNode
> (regression/javascript/tree/tree_spec.js:465:116) at
> UserContext. (regression/javascript/tree/tree_spec.js:790:14)
> TypeError: Cannot read property 'reload' of undefined at
> UserContext.
> (regression/javascript/tree/tree_spec.js:798:16)HeadlessChrome 0.0.0 (Mac
> OS X 10.11.6): Executed 535 of 586 (2 FAILED) (0 secs / 10.365
> secs)HeadlessChrome 0.0.0 (Mac OS X 10.11.6) tree tests TreeNode #reload
> reloads the node and its children FAILED TypeError: Cannot read property
> 'fakeFail' of undefined at TreeFake.addNewNode
> (regression/javascript/tree/tree_spec.js:237:22) at TreeFake.addNewNode
> (regression/javascript/tree/tree_spec.js:465:116) at
> UserContext. (regression/javascript/tree/tree_spec.js:790:14)
> TypeError: Cannot read property 'reload' of undefined...*
>
>
> After applying the proposed patch:
>
> *...*
> *HeadlessChrome 0.0.0 (Mac OS X 10.11.6) tree tests TreeNode #hasParent
> parent exists returns true FAILED*
> * Expected true to be false.*
> * at UserContext.
> (regression/javascript/tree/webpack:/regression/javascript/tree/tree_spec.js:233:40)*
> *HeadlessChrome 0.0.0 (Mac OS X 10.11.6): Executed 534 of 586 (1 FAILED)
> (0 secs / 8.157 secs)*
> *HeadlessChrome 0.0.0 (Mac OS X 10.11.6) tree tests TreeNode #hasParent
> parent exists returns true FAILED*
> * Expected true to be false.*
> *HeadlessChrome 0.0.0 (Mac OS X 10.11.6) tree tests TreeNode #reload
> reloads the node and its children FAILED*
> * TypeError: Cannot read property 'fakeFail' of undefined*
> * at TreeFake.addNewNode
> (regression/javascript/tree/webpack:/pgadmin/static/js/tree/tree.js:96:18)*
> * at TreeFake.addNewNode
> (regression/javascript/tree/webpack:/regression/javascript/tree/tree_fake.js:25:5)*
> * at UserContext.
> (regression/javascript/tree/webpack:/regression/javascript/tree/tree_spec.js:243:14)*
> * TypeError: Cannot read property 'reload' of undefined*
> * at UserContext.
> (regression/javascript/tree/webpack:/regression/javascript/tree/tree_spec.js:252:16)*
> *HeadlessChrome 0.0.0 (Mac OS X 10.11.6): Executed 535 of 586 (2 FAILED)
> (0 secs / 8.179 secs)*
> *HeadlessChrome 0.0.0 (Mac OS X 10.11.6) tree tests TreeNode #reload
> reloads the node and its children FAILED*
> * TypeError: Cannot read property 'fakeFail' of undefined*
> * at TreeFake.addNewNode
> (regression/javascript/tree/webpack:/pgadmin/static/js/tree/tree.js:96:18)*
> * at TreeFake.addNewNode
> (regression/javascript/tree/webpack:/regression/javascript/tree/tree_fake.js:25:5)*
> * at UserContext.
> (regression/javascript/tree/webpack:/regression/javascript/tree/tree_spec.js:243:14)*
> * TypeError: Cannot read property 'reload' of undefined*
>
> *...*
>
>
>
> As you can see, it gives a lot more relevant information in the stack
> trace.
>
> Please let me know if you have any objection to the proposed patch.
>
> --
>
> Thanks & Regards,
>
> Ashesh Vashi
> EnterpriseDB INDIA: Enterprise PostgreSQL Company
> 
>
>
> *http://www.linkedin.com/in/asheshvashi*
> 
>

Re: [pgAdmin4][RM#3289] Can't query SQL_ASCII database.

[Joao De Almeida Pereira]

Hello Aditya,

It looks ok and it passes CI.

We have some recommendations:
- These look like 2 different changes so they should be in separated commits
- Do we have test coverage for the bug that you are talking about? If not
we should, to ensure this problem does not happen again in a future change.


Thanks
Victoria && Joao

On Wed, May 30, 2018 at 3:06 AM Aditya Toshniwal <
aditya.toshni...@enterprisedb.com> wrote:

> Hi Hackers,
>
> PFA updated patch after all the permutations, combinations for encoding
> for SQL_ASCII database.  Also fixed a small glitch for sql editor
> connection status check.
>
> Please note, ERROR: invalid byte sequence for encoding "UTF8": 0xe9 0x73
> is a Postgres DB error and not pgAdmin4 error.
>
>
>
> You need to change client_encoding to the appropriate. After changing
> client_encoding using command - set client_encoding='XYZ', it will give not
> give error.
>
>
>
>
>
>
>
>
> Thanks and Regards,
> Aditya Toshniwal
> Software Engineer | EnterpriseDB Software Solutions | Pune
> "Don't Complain about Heat, Plant a tree"
>
> On Wed, May 23, 2018 at 10:13 AM, Aditya Toshniwal <
> aditya.toshni...@enterprisedb.com> wrote:
>
>> Thank you Victoria, Anthony.
>>
>> Thanks and Regards,
>> Aditya Toshniwal
>> Software Engineer | EnterpriseDB Software Solutions | Pune
>> "Don't Complain about Heat, Plant a tree"
>>
>> On Tue, May 22, 2018 at 7:15 PM, Victoria Henry 
>> wrote:
>>
>>> Hi Aditya,
>>>
>>> We made a minor change to make the patch so the python linter can pass.
>>> Attached is the change we made.
>>> Everything else looks good.
>>>
>>> Sincerely,
>>>
>>> Victoria & Anthony
>>>
>>> On Tue, May 22, 2018 at 4:46 AM Aditya Toshniwal <
>>> aditya.toshni...@enterprisedb.com> wrote:
>>>
 Hi,

 PFA updated patch. Linter issues are fixed ( we dont have any linter
 setup for python :-( )
 Regarding test cases, they run successfully on my system and the reason
 it failed for pivotal is timeout exception. I am sorry I can't help with
 that.

 Traceback (most recent call last):
   File
 "/tmp/build/a453582b/pgadmin-repo/web/pgadmin/feature_tests/keyboard_shortcut_test.py",
 line 52, in runTest
 self._check_shortcuts()
   File
 "/tmp/build/a453582b/pgadmin-repo/web/pgadmin/feature_tests/keyboard_shortcut_test.py",
 line 77, in _check_shortcuts
 ") and contains(@class, 'open')]")
   File
 "/root/.pyenv/versions/pgadmin36/lib/python3.6/site-packages/selenium/webdriver/support/wait.py",
 line 80, in until
 raise TimeoutException(message, screen, stacktrace)
 selenium.common.exceptions.TimeoutException: Message:

 Thanks and Regards,
 Aditya Toshniwal
 Software Engineer | EnterpriseDB Software Solutions | Pune
 "Don't Complain about Heat, Plant a tree"

 On Tue, May 22, 2018 at 1:37 PM, Dave Page  wrote:

> Hi
>
> Pivotal's buildbot is showing problems with this patch:
>
>
> https://gpdb-dev.bosh.pivotalci.info/teams/pgadmin/pipelines/pgadmin-patch/jobs/run-linter/builds/66
> (linter failed)
>
> https://gpdb-dev.bosh.pivotalci.info/teams/pgadmin/pipelines/pgadmin-patch/jobs/run-tests/builds/84
> (tests failed)
>
>
> On Tue, May 22, 2018 at 7:05 AM, Aditya Toshniwal <
> aditya.toshni...@enterprisedb.com> wrote:
>
>> Hi Hackers,
>>
>> PFA patch for RM#3289 where decode error was thrown on querying a
>> SQL_ASCII database table. Please note, this problem occurs only on 
>> windows.
>> Sample insert - insert into test_tab values ('é');
>>
>> psycopg2 has a encodings dictionary where Postgres Database Encodings
>> are mapped to python equivalent. It uses 'ascii' decoder of python to
>> decode for SQL_ASCII encoding. If data has characters beyond the limit of
>> ascii then it failed. The solution would be to use utf_8 decoder instead 
>> of
>> ascii. I tried setting the client_encoding using
>> set_client_encoding('UTF8') method of a psycopg2 connection but no luck
>> (also its not allowed for async connection). I also tried executing "SET
>> CLIENT_ENCODING='UTF8'" but it didn't work too.
>> So, as in the patch, I had to set encodings dict value directly to
>> 'utf_8' and it seems to be working. Please note, the same is added to
>> psycopg3 milestones
>> https://github.com/psycopg/psycopg2/milestone/4
>>
>> Also fixed a small glitch for sql editor connection status check.
>>
>> Kindly review.
>>
>> Thanks and Regards,
>> Aditya Toshniwal
>> Software Engineer | EnterpriseDB Software Solutions | Pune
>> "Don't Complain about Heat, Plant a tree"
>>
>
>
>
> --
> Dave Page
> Blog: http://pgsnake.blogspot.com
> Twitter: @pgsnake
>
> EnterpriseDB UK: http://www.enterprisedb.com
> The Enterprise PostgreSQL Company
>


>>
>

Re: [pgAdmin4][Patch] RM #3355 User can not perform operation of backup,Backup all, Backup global and Maintenance DB with ssh tunneling

[Joao De Almeida Pereira]

Hello Akshay,

Looks like the tests failed, but it is just flakes. We assume no regression
was made with this code, despite the fact that no tests were added to
ensure the new code is working.

Thanks
Anthony && Joao

On Tue, May 29, 2018 at 6:41 AM Akshay Joshi 
wrote:

> Hi Hackers,
>
> Attached is the patch to fix RM #3355 User can not perform operation of
> backup, Backup all, Backup global and Maintenance DB with ssh tunneling.
>
> Please review it.
>
> --
> *Akshay Joshi*
>
> *Sr. Software Architect *
>
>
>
> *Phone: +91 20-3058-9517Mobile: +91 976-788-8246*
>

Re: [pgAdmin4][Patch] Feature #3204 Notify/Listen not working in version 2.1

[Joao De Almeida Pereira]

Hello Akshay,

The code looks good, we do have some minor notes for next time:

. To make the code easier to understand we should decide on 1 term and not
have notifies and notifications to reference the same thing.
. It would be nice if we tried not to use XPATH to get the HTML components
that we are testing on. Maybe try to use CSS classes. Selenium also has a
driver.select_by_class_name which would work well in this case.

The error that we are seeing in CI is related to pg_notify. This function
was introduced in 9.0 and Greenplum is still not there yet. In order to
solve this need to skip that part of the tests. There is an example in that
same file using the function _test_explain_plan_feature to skip a tests
depending on the Version.

Thanks
Anthony && Joao
​

On Mon, May 28, 2018 at 1:34 AM Akshay Joshi 
wrote:

> Hi Hakers,
>
> On my last patch feature tests were failed for GreenPlum5 database only
> not for PostgreSQL. I have verified that on
> https://gpdb-dev.bosh.pivotalci.info/teams/pgadmin/pipelines/pgadmin-patch/jobs/run-tests/builds/93
> .
>
> Can someone from Pivotal team help me, as I don't have GreenPlum database
> and don't know why it is failing.
>
> On Sat, May 26, 2018 at 5:47 PM, Akshay Joshi <
> akshay.jo...@enterprisedb.com> wrote:
>
>> Hi Hackers,
>>
>> As per suggestion by Dave and discussion with in the team, I have
>> modified the logic again. Following are the modifications:
>>
>>- Instead of waiting for another query to execute on the session
>>where 'LISTEN' command has been executed, we fetched the notify messages 
>> in
>>the connection status polling logic. Doing this user will get the notify
>>messages asap.
>>- Instead of showing all the notifications in single alertify dialog,
>>we call *alertify.info ('') *for
>>individual notifications.
>>- Created new tab 'Notifications' in Query Tool where all the notify
>>messages will be recorded with the timestamp and payload.
>>
>> Please review the latest patch.
>>
>> On Tue, May 22, 2018 at 2:43 PM, Dave Page  wrote:
>>
>>>
>>>
>>> On Tue, May 22, 2018 at 10:01 AM, Akshay Joshi <
>>> akshay.jo...@enterprisedb.com> wrote:
>>>
 Hi Dave

 On Tue, May 22, 2018 at 2:02 PM, Dave Page  wrote:

>
>
> On Tue, May 22, 2018 at 9:13 AM, Dave Page  wrote:
>
>> Hi
>>
>> On Tue, May 22, 2018 at 7:07 AM, Akshay Joshi <
>> akshay.jo...@enterprisedb.com> wrote:
>>
>>> Hi Hackers,
>>>
>>> As per suggestion by Dave, I have modified the logic and now
>>> notifications are popped up in alertify dialog(refer
>>> Notify_Messages.png) as and when received on that session where
>>> "LISTEN" is executed. Attached is the modified patch, please review it.
>>>
>>> To test this feature following steps need to perform:
>>>
>>>- Apply the patch.
>>>- Run pgAdmin4
>>>- Connect to any database server and open query tool.
>>>- Execute 'LISTEN foo;' command.
>>>- Open another query tool window and execute 'NOTIFY foo'. (This
>>>is without payload).
>>>- Execute 'select pg_notify('foo', 'Hello')' query (with
>>>payload).
>>>- Go to the query tool window from where 'LISTEN' was executed
>>>and run any other query.
>>>
>>> I think there was a small misunderstanding here - I was suggesting
>> that each notification be displayed in an Alertify notification, e.g. 
>> using
>> alertify.message('A notification of FOO was received with payload
>> '1234'...')
>>
>
If there are too many notifications then it's annoying for user
 to popped up N number of alertify dialogs. Notification is only
 receives when any other query execute on the session where "LISTEN" command
 executes. So for example I have NOTIFY 10 times from different sessions and
 execute any other query on the session("LISTEN" one), 10 alertify
 dialog will be popped up.

>>>
>>> Sure, but then a) it's the users choice to listen for something very
>>> noisey, and b) if there are many notifications then the message box
>>> dialogue will become huge.
>>>
>>> The other nice thing about using notifications is that they don't
>>> require any acknowledgement; they show you the event happened, and then get
>>> out of the way, allowing you to jump to the messages tab if needed.
>>>
>>>


>
> And it failed tests:
> https://gpdb-dev.bosh.pivotalci.info/teams/pgadmin/pipelines/pgadmin-patch/jobs/run-tests/builds/85
> :-(
>
> Again it's timeout issue, not able to reproduce on my machine will
 look into it. Maybe will have to add webDriverWait.

>>>
>>> OK.
>>>
>>>

>>
>>
>>>
>>>-
>>>
>>>
>>>
>>>
>>> On Mon, May 21, 2018 at 1:36 PM, Akshay Joshi <
>>> akshay.jo...@enterprisedb.com> wrote:
>>>
 Hi Dave

 On Fri, 

Re: [pgadmin4][patch] Use pytest test runner for unit tests

[Joao De Almeida Pereira]

Hello Dave

> As part of the development environment we do not see the reasoning behind
>> not add PYTHONPATH to the environment variables, specially because this
>> looks like the way pytest was invisoned.
>>
>
> Really? It's one more step that wasn't previously required, and for which
> there is no good reason when running in a properly configured virtual
> environment. Not only that, but PYTHONPATH is typically used as a search
> path to find modules on which the application/script is dependent - not to
> find the application/script itself. Unconditionally overriding it is likely
> to lead to problems in some possible configurations (at the very least I
> would expect to see the Makefile do PYTHONPATH=$PYTHONPATH:$(PWD)/web).
>
Good point we didn't consider the possibility of someone developing
multiple python apps in the same machine.
We will revisit this and start using
python -m pytest
as that should solve the problem

>
>
>>
>> However please try the following patch instead. We've changed the pytest
>> invocation to assume the relevant dir as part of the directories to load,
>> as well as the docs and Makefile
>>
>
> Some initial feedback:
>
> - The JSON results are no longer being generated.
>

That is a flag that we didn't add to the script bug we will review the
patch with that


> - The output is *extremely* verbose, showing a lot of seemingly
> unnecessary info (do we really need to output the source code of failing
> test cases?). I would suggest that the verbose output be directed to the
> log, and the visible output should be much more terse along the lines of
> what it is now.
>

We can see what we can do about this point


> - There is no summary at the end showing what passed, failed or was
> skipped. To figure out what to look at, I have to trawl through over 13K
> lines of output (642KB).
>

If we use the flag -q the output is smaller. We can add that too


> - 69 tests failed for me running test:unit. They were previously all
> passing.
>

Can you provide some log of the failing tests?


> - It is a *lot* faster - not sure if that's a result of tests failing, but
> I expect not entirely.
>
> - /README was updated, but not /web/regression/README
>

Out lapse, will do that

Joao

Re: Nightly snapshot builds

[Joao De Almeida Pereira]

Hi Dave
That is huge news, this takes us closer to a more frequent cadence of
releases and that is very exciting prospect.

Thanks for all the hard work to make this change.

Best regards
Joao

On Fri, May 25, 2018, 6:34 AM Ashesh Vashi 
wrote:

> Awesome - Dave!
>
> --
>
> Thanks & Regards,
>
> Ashesh Vashi
> EnterpriseDB INDIA: Enterprise PostgreSQL Company
> 
>
>
> *http://www.linkedin.com/in/asheshvashi*
> 
>
> On Fri, May 25, 2018 at 4:01 PM, Dave Page  wrote:
>
>> All,
>>
>> I'm pleased to say that we now have automated nightly snapshot builds
>> (Windows, macOS, Python Wheel and source code) available at:
>> https://www.postgresql.org/ftp/pgadmin/pgadmin4/snapshots/
>>
>> If you're using one of these, please bear in mind that they have had no
>> QA (beyond automated and developer testing at commit time). If reporting an
>> issue, please be clear which snapshot you're using.
>>
>> The system will keep the 5 most recent builds at any time.
>>
>> Thanks!
>>
>> --
>> Dave Page
>> Blog: http://pgsnake.blogspot.com
>> Twitter: @pgsnake
>>
>> EnterpriseDB UK: http://www.enterprisedb.com
>> The Enterprise PostgreSQL Company
>>
>
>

Re: [pgAdmin4][RM#3271] Update to the latest 3.x version of jQuery

[Joao De Almeida Pereira]

Hello Aditya,

We changed the way the tests were running and we could remove the function.
Instead of piggybackiung on a deprecated field we created a html fixture

$('body').append(
  '' +
  '' +
  '' +
  ''
);


With it we could use the real JQuery instead of mocking it.
Now the tests look like this

it('disables the run query button', () => {
  let buttonFlash = $('#btn-flash');

  expect(buttonFlash.prop('disabled')).toEqual(true);
});



Thanks
Anthony & Joao

On Thu, May 24, 2018 at 12:26 AM Aditya Toshniwal <
aditya.toshni...@enterprisedb.com> wrote:

> Hi,
>
> No luck with data-test-selector. :(
> I added data-test-selector to html tags. I get undefined for
> .data('test-selector') as well as for .attr('data-test-selector').
>
> Thanks and Regards,
> Aditya Toshniwal
> Software Engineer | EnterpriseDB Software Solutions | Pune
> "Don't Complain about Heat, Plant a tree"
>
> On Thu, May 24, 2018 at 9:17 AM, Aditya Toshniwal <
> aditya.toshni...@enterprisedb.com> wrote:
>
>> Hi Victoria/Joao,
>>
>> Thank you for reviewing. The above function is probably the last solution
>> which I added.
>> The test cases
>> in web/regression/javascript/sqleditor/execute_query_spec.js calls the
>> execute query function of pgAdmin4 sqleditor. The execution
>> disables/enables the buttons internally and is not done by the test cases.
>> So there is spy on prop function (jqueryPropSpy = spyOn($.fn, 'prop')) of
>> jQuery. The problem is, the spy jQuery object (in function
>> findJQueryCallWithSelector) returned seems to have very less information,
>> not sure why. I had tried using css selector for id, tried extracting id
>> directly, class, but none of them were present in the object.
>>
>> I can try adding data-test-selector to the HTML5 tag, if it works then
>> will send the updated patch.
>>
>>
>> Thanks and Regards,
>> Aditya Toshniwal
>> Software Engineer | EnterpriseDB Software Solutions | Pune
>> "Don't Complain about Heat, Plant a tree"
>>
>> On Wed, May 23, 2018 at 7:41 PM, Joao De Almeida Pereira <
>> jdealmeidapere...@pivotal.io> wrote:
>>
>>> HI Aditya,
>>>
>>> Good job porting the app from the ancient version of JQuery to the
>>> latest build :D
>>>
>>> The patch in general looks good, and the patches-bot says all tests
>>> pass.
>>>
>>> The only thing that we would like to point out is this piece of code:
>>>
>>> /* jQuery has removed selector property in version 3.x
>>>  * To allow JS test cases to run, the below function is
>>>  * used to set the selector property
>>>  */
>>> function setPropAndSelector(selector, propName, propVal) {
>>>   let tmpObj = $(selector);
>>>   tmpObj['selector'] = selector;
>>>   tmpObj.prop(propName,propVal);
>>>   return tmpObj;
>>> }
>>>
>>> As tests are also part of the code we believe that when there is a
>>> change that affect testing we should change the tests to work accordingly.
>>> So our suggestion is, instead of creating this function, why don’t we use a
>>> CSS selector in the tests to do the clicking or even use the HTML5 tag
>>> data-test-selector?
>>> This way the tests will also be coherent with the upgrade from JQuery 1
>>> to 3
>>> ​
>>>
>>> Thanks
>>> Victoria & Joao
>>>
>>> On Wed, May 23, 2018 at 6:44 AM Aditya Toshniwal <
>>> aditya.toshni...@enterprisedb.com> wrote:
>>>
>>>> Hi Hackers,
>>>>
>>>> Please find the updated patch for upgrading the jQuery version to 3.x.
>>>> The patch is only for pgAdmin4 code changes replacing old deprecated
>>>> functions with new replacement.
>>>> We need to look into external libraries used for any vulnerabilities.
>>>> Request you to kindly review.
>>>>
>>>> Thanks and Regards,
>>>> Aditya Toshniwal
>>>> Software Engineer | EnterpriseDB Software Solutions | Pune
>>>> "Don't Complain about Heat, Plant a tree"
>>>>
>>>> On Mon, May 14, 2018 at 1:23 PM, Aditya Toshniwal <
>>>> aditya.toshni...@enterprisedb.com> wrote:
>>>>
>>>>> Hi Hackers,
>>>>>
>>>>> Please hold on with the patch. Will send the updated patch soon.
>>>>> Apologies.
>>>>>
>>>>>
>>>>>
>>>>> Thanks and Regards,
>>>>> Aditya Toshniwal
>>>>&g

Re: [pgadmin4][patch] Initial patch to decouple from ACI Tree

[Joao De Almeida Pereira]

Hey, Thanks so much for the reply.

We've noticed that you've made several modifications on top of our original
patch. Unfortunately, we've found it very hard to follow. Could we please
get a brief synopsis of the changes you have made - just so we can better
understand the rationale behind them? Just like we've done for you
previously.

Let's keep in mind that the original intent was simply to introduce this
abstraction into the code base, which is a big enough task. I'd hate for
the scope of the changes we're making to expand beyond that.

Thanks
Joao && Anthony


On Thu, May 24, 2018 at 2:59 AM Ashesh Vashi 
wrote:

> Sorry for the late reply.
> On Wed, May 16, 2018 at 8:55 PM, Anthony Emengo 
> wrote:
>
>> export function canCreate(pgBrowser, childOfCatalogType) {
>>   return canCreateObject.bind({
>> browser: pgBrowser,
>> childOfCatalogType: childOfCatalogType,
>>   });
>> }
>>
>> With respect to the above code: this bind pattern looks good and seems
>> like the idiomatic way to handle this in JavaScript. On a lighter node, I
>> don’t even see the need for an additional method to wrap it. The invocation
>> could have easily been like canCreate: canCreateObject.bind({ browser:
>> pgBrowser, childOfCatalogType: childOfCatalogType }), I don’t feel too
>> strongly here.
>>
> I do agree - we can handle the same problem many ways.
> I prefer object oriented pardigm more in general.
> Any way - I have modified the code with some other changes.
>
>> I renamed it as isValidTreeNodeData, because - we were using it in for
>> testing the tree data. Please suggest me the right place, and name.
>>
>> We’re not sure; maybe after continued refactoring, we will come across
>> more generic functions. At that point we can revisit this and create a
>> utils.js file.
>>
> Sure.
>
>> The original patch was separating them in different places, but - still
>> uses some of the functionalities directly from the tree, which was
>> happening because we have contextual menu.
>> To give a better solution, I can think of putting the menus related code
>> understand ‘sources/tree/menu’ directory.
>>
>> We’re particularly worried because we’re trying to avoid the coupling
>> that we see in the code base today. We want to decouple *application
>> state* from *business domain* logic as much as we can - because this
>> makes the code much easier to understand. We achieve lower coupling by have
>> more suitable interfaces to retrieve *application state* like: anyParent
>> (the menu doesn’t care how this happens). This is the direction that we’re
>> trying to move towards, we just don’t want the package structure to
>> undermine that developer intent.
>>
> I realized after revisiting the code, menu/can_create.js was only
> applicable to the children of the schema/catalog nodes, same as
> 'can_drop_child'.
> We should have put both scripts in the same directory.
>
> Please find the updated patch for the same.
>
> Please review it, and let me know your concerns.
>
> -- Thanks, Ashesh
>
>> How about nodeMenu.isSupportedNode(…)?
>>
>> Naming is one of the hardest problems in programming. I don’t feel too
>> strongly about this one. For now, let’s keep it as is
>>
>> Thanks
>> Anthony && Victoria
>> ​
>>
>>
>>
>>
>

Re: [pgadmin4][patch] Initial patch to decouple from ACI Tree

[Joao De Almeida Pereira]

Hello Hackers,

Just to remind that if you need some more information from our side you can
ping us.
Can we expect and email tomorrow morning with the commit?

Thanks
Victoria & Joao

On Mon, May 21, 2018 at 9:56 AM Dave Page  wrote:

> Ashesh, please prioritise this so we can move on.
>
> Thanks.
>
> On Mon, May 21, 2018 at 2:46 PM, Anthony Emengo 
> wrote:
>
>> Hey all,
>>
>> We haven't heard from you all in a while regarding our last statements.
>> Is there any thing that I need to clarify? We feel left in the dark here
>> and just want to know what we can do help.
>>
>> Cheers!
>> Anthony && Joao
>>
>
>
>
> --
> Dave Page
> Blog: http://pgsnake.blogspot.com
> Twitter: @pgsnake
>
> EnterpriseDB UK: http://www.enterprisedb.com
> The Enterprise PostgreSQL Company
>

Re: [pgAdmin4][RM#3271] Update to the latest 3.x version of jQuery

[Joao De Almeida Pereira]

HI Aditya,

Good job porting the app from the ancient version of JQuery to the latest
build :D

The patch in general looks good, and the patches-bot says all tests pass.

The only thing that we would like to point out is this piece of code:

/* jQuery has removed selector property in version 3.x
 * To allow JS test cases to run, the below function is
 * used to set the selector property
 */
function setPropAndSelector(selector, propName, propVal) {
  let tmpObj = $(selector);
  tmpObj['selector'] = selector;
  tmpObj.prop(propName,propVal);
  return tmpObj;
}

As tests are also part of the code we believe that when there is a change
that affect testing we should change the tests to work accordingly. So our
suggestion is, instead of creating this function, why don’t we use a CSS
selector in the tests to do the clicking or even use the HTML5 tag
data-test-selector?
This way the tests will also be coherent with the upgrade from JQuery 1 to 3
​

Thanks
Victoria & Joao

On Wed, May 23, 2018 at 6:44 AM Aditya Toshniwal <
aditya.toshni...@enterprisedb.com> wrote:

> Hi Hackers,
>
> Please find the updated patch for upgrading the jQuery version to 3.x. The
> patch is only for pgAdmin4 code changes replacing old deprecated functions
> with new replacement.
> We need to look into external libraries used for any vulnerabilities.
> Request you to kindly review.
>
> Thanks and Regards,
> Aditya Toshniwal
> Software Engineer | EnterpriseDB Software Solutions | Pune
> "Don't Complain about Heat, Plant a tree"
>
> On Mon, May 14, 2018 at 1:23 PM, Aditya Toshniwal <
> aditya.toshni...@enterprisedb.com> wrote:
>
>> Hi Hackers,
>>
>> Please hold on with the patch. Will send the updated patch soon.
>> Apologies.
>>
>>
>>
>> Thanks and Regards,
>> Aditya Toshniwal
>> Software Engineer | EnterpriseDB Software Solutions | Pune
>> "Don't Complain about Heat, Plant a tree"
>>
>> On Mon, May 14, 2018 at 12:52 PM, Aditya Toshniwal <
>> aditya.toshni...@enterprisedb.com> wrote:
>>
>>> Hi Hackers,
>>>
>>> PFA patch for upgrading jQuery version to 3.3.1 from current version
>>> 1.12.4. Patch includes replacing deprecated functions of jquery to latest
>>> one.
>>> Kindly review.
>>>
>>> Thanks and Regards,
>>> Aditya Toshniwal
>>> Software Engineer | EnterpriseDB Software Solutions | Pune
>>> "Don't Complain about Heat, Plant a tree"
>>>
>>
>>
>

Re: Implement geospatial data viewer in pgAdmin4 for PostGIS

[Joao De Almeida Pereira]

Hello Xuri,
Welcome to the pgadmin project.

Let us give you some guidelines/pointers to help you out starting.
As a general rule of thumb all the new development should be done in a
separate file that contain the functionality itself.
For Javascript,
- Has you might have notice we started some time ago the migration from
RequireJS into Webpack and ES6 for a more standardized way to develop and
we are trying no to touch the files are not ported yet.
You will need to connect the new functionality and will need to change
these files, but for that we would advise you to do something similar to
some code we have in SQLEditor:

_show_filter: function() {
  let self = this;
  FilterHandler.dialog(self);
},

- Create tests around your functionality
- If you are developing a panel like the History one in SQLEditor, do you
tthink you can use React to do it? History is an example of that
- We use ESLinter + Jasmine + Karma for linting and testing the javascript
side, be sure to run these before sending a patch

For Python:
- As advised in the Javascript part we are trying not to increase the
length of the current files as much as possible, so develop you backend in
a separate file(s) and then integrate them as normal classes of functions
in Python
- We use pycodestyle + Unittest for testing and linting, be sure to run
these before sending a patch
- Add tests around your functionality


If you need some more pointer or help in something let us know.

Thanks
Joao

On Wed, May 23, 2018 at 8:23 AM Xuri Gong  wrote:

> Hi Dave/Team,
>
> My name is Xuri Gong. I am working on implementing geospatial data viewer
> in pgAdmin4.
>
> I wrote a design document for the viewer[1] according to the related
> issue[2] and I am wondering if it is a proper design. It would be great if
> there are any suggestions.
>
> I have read the #pgAdmin Project Contributions# doc[3]. Is there any other
> instruction or standard that is needed to learn?
>
> [1]
> https://docs.google.com/document/d/1NE1RLTp9uw9fgduZEerrKKbQZEJCPyrSyQKa0-BDZsI/edit?usp=sharing
> [2] https://redmine.postgresql.org/issues/1407
> [3] https://www.pgadmin.org/docs/pgadmin4/dev/contributions.html
>
> Regards,
> Xuri Gong
>

Re: [pgadmin4][patch] Initial patch to decouple from ACI Tree

[Joao De Almeida Pereira]

Hello Ashesh,

These are our comments to the patch:


   -

   Can you explain the reasoning behind the change you did on the canCreate
   function?
   Bind creates a new instance of a function, do we really need that?
   -

   isValidTreeNodeData
   1. If it is not Null or Undefined it really means that the data is
  Valid? Shouldn’t it be isDataDefined?
  2. This looks like a generic function that could be used with objects
  of any type not specific to TreeNodeData. So the file location
doesn’t look
  correct.
   -

   The tree folder is just a Tree that we use to store information. The
   menu uses a Tree but the 2 things should be separated.
   In our point of view the current entanglement of the ACITree into our
   code came from missing concepts into a single place (Menu + Storage of
   information).
   The idea behind having the Tree as a separate block was to ensure that
   we do not have the Menu and Tree coupling.
   -

   supportedNodesMenu.enabled what it does it check if a Node Menu should
   be enabled or not. The name of it maybe should be nodeMenu.enabled?

​


Thanks
Victoria & Joao

On Tue, May 15, 2018 at 6:37 AM Ashesh Vashi <ashesh.va...@enterprisedb.com>
wrote:

> Hi Joao,
> On Mon, May 14, 2018 at 6:11 PM, Ashesh Vashi <
> ashesh.va...@enterprisedb.com> wrote:
>
>>
>> On Mon, May 14, 2018 at 6:10 PM, Dave Page <dp...@pgadmin.org> wrote:
>>
>>>
>>>
>>> On Mon, May 14, 2018 at 1:38 PM, Ashesh Vashi <
>>> ashesh.va...@enterprisedb.com> wrote:
>>>
>>>> On Mon, May 14, 2018 at 2:59 PM, Dave Page <dp...@pgadmin.org> wrote:
>>>>
>>>>>
>>>>>
>>>>> On Sat, May 12, 2018 at 12:10 AM, Ashesh Vashi <
>>>>> ashesh.va...@enterprisedb.com> wrote:
>>>>>
>>>>>> On Sat, May 12, 2018, 02:58 Joao De Almeida Pereira <
>>>>>> jdealmeidapere...@pivotal.io> wrote:
>>>>>>
>>>>>>> Hello Ashesh,
>>>>>>>
>>>>>>> 1. In TreeNode, we're keepging the reference of DOMElement, do we
>>>>>>>>> really need it?
>>>>>>>>
>>>>>>>> As of right now, our Tree abstraction acts as an adapter to the
>>>>>>>>> aciTree library. The aciTree library needs the domElement for most of 
>>>>>>>>> its
>>>>>>>>> functions (setInode, unload, etc). Thus this is the easiest way to
>>>>>>>>> introduce our abstraction and keep the functionality as before - at 
>>>>>>>>> least
>>>>>>>>> until we decide that whether we want to switch out the library or not.
>>>>>>>>
>>>>>>>> I understand that. But - I've not seen any reference of domElement
>>>>>>>> the code yet, hence - pointed that out.
>>>>>>>
>>>>>>> If you look at the function: reload, unload you will see that
>>>>>>> domNode is used to communicate with the ACITree
>>>>>>> ​
>>>>>>>
>>>>>>>> 2. Are you expecting the tree class to be a singleton class
>>>>>>>>
>>>>>>>> Since this tree is referenced throughout the codebase, considering
>>>>>>>>> it to be a singleton seems like the most appropriate pattern for this
>>>>>>>>> usecase. It is very much the same way how we create a single instance 
>>>>>>>>> of
>>>>>>>>> the aciTree library and use that throughout the codebase. Moreover, it
>>>>>>>>> opens up opportunities to improve performance, for example caching 
>>>>>>>>> lockups
>>>>>>>>> of nodes. I’m not a fan of singletons myself, but I feel like we’re 
>>>>>>>>> simply
>>>>>>>>> keeping the architecture the same in the instance.
>>>>>>>>
>>>>>>>> Yeah - I don't see any usage of tree object from anywhere.
>>>>>>>> And, we're already creating new object in browser.js (and, not
>>>>>>>> utitlizing that instance anywhere.)
>>>>>>>
>>>>>>>
>>>>>>> You are right, we do not need to export tree as a singleton for now.
>>>>>>> The line that exports the variable tree can be remove when applying
>>>>>>> the patch number 2.
>>>>>>> ​
>>>>>>>
>>>>>>> I think we addressed all the concern raised about this patch. Does
>>>>>>> this mean that the patch is going to get committed?
>>>>>>>
>>>>>> Yes - from me for 0002.
>>>>>>
>>>>>
>>>>> Can you do that today?
>>>>>
>>>> Done.
>>>>
>>>
>>> Great, thanks!
>>>
>>> On to patch 0003 then :-)
>>>
>> Yes - already working on it! :-)
>>
> Majority part of the 0003 patch looks good to me.
> Except choice of the path of some of the file, and name of the functions.
>
> Please find the updated patch.
> I've moved files under the 'pgadmin/static/js/menu' directory under the
> 'pgadmin/static/js/tree', as they're using tree functionalities directly.
>
> Please review it, and let me know your concern.
>
> -- Thanks, Ashesh
>
>>
>> -- Thanks, Ashesh
>>
>>>
>>> --
>>> Dave Page
>>> Blog: http://pgsnake.blogspot.com
>>> Twitter: @pgsnake
>>>
>>> EnterpriseDB UK: http://www.enterprisedb.com
>>> The Enterprise PostgreSQL Company
>>>
>>
>>
>

Re: [pgAdmin4][Patch] Feature #3204 Notify/Listen not working in version 2.1

[Joao De Almeida Pereira]

Hi Akshay,

This patch is flaky; it doesn't always pass the tests in our pipeline.

==
 

ERROR: runTest (pgadmin.feature_tests.query_tool_tests.QueryToolFeatureTest)
 

Query tool feature test
 

--
 

Traceback (most recent call last):
 

  File 
"/tmp/build/5988fb0a/pgadmin-repo/web/pgadmin/feature_tests/query_tool_tests.py",
line 101, in runTest
 

self._query_tool_notify_statements()
 

  File 
"/tmp/build/5988fb0a/pgadmin-repo/web/pgadmin/feature_tests/query_tool_tests.py",
line 643, in _query_tool_notify_statements
 

'//div[contains(@class, "sql-editor-message") and '
 

  File 
"/tmp/build/5988fb0a/pgadmin-repo/web/regression/feature_utils/pgadmin_page.py",
line 169, in find_by_xpath
 

lambda driver: driver.find_element_by_xpath(xpath)
 

  File 
"/tmp/build/5988fb0a/pgadmin-repo/web/regression/feature_utils/pgadmin_page.py",
line 261, in wait_for_element
 

return self._wait_for("element to exist", element_if_it_exists)
 

  File 
"/tmp/build/5988fb0a/pgadmin-repo/web/regression/feature_utils/pgadmin_page.py",
line 327, in _wait_for
 

"Timed out waiting for " + waiting_for_message
 

  File 
"/root/.pyenv/versions/pgadmin/lib/python2.7/site-packages/selenium/webdriver/support/wait.py",
line 80, in until
 

raise TimeoutException(message, screen, stacktrace)
 

TimeoutException: Message: Timed out waiting for element to exist


All the failures are related to query_tool_notify_statements.  Please take
another look.

Sincerely,

Victoria & Joao

On Tue, May 15, 2018 at 6:01 AM Akshay Joshi 
wrote:

> Hi Hackers,
>
> Attached is the patch to capture the notification from psycopg2 and
> displayed it in "Messages" tab of query tool. Added feature test to cover
> this scenario.
>
> Refer Notification.png file to how it looks in "Messages" tab. Please
> review it.
>
> --
> *Akshay Joshi*
>
> *Sr. Software Architect *
>
>
>
> *Phone: +91 20-3058-9517Mobile: +91 976-788-8246*
>

Re: [pgAdmin4][Patch]: Minor JS Fix

[Joao De Almeida Pereira]

Hi,
The code looks fine, all tests pass and linter.

Once again, not to sound like a broken record, we could have done some much
more to improve our code base
Why didn't we extract that function into a more testable place?

In order for the code to become more clean and modular, we should try to
always write tests whenever we make changes to the code, even if that means
extracting a small portion (ie. the onLoad function)

Thanks
Victoria & Joao

On Tue, May 15, 2018 at 12:44 AM Khushboo Vashi <
khushboo.va...@enterprisedb.com> wrote:

> Hi,
>
> Please find the attached monir patch to fix the JS issue.
>
> Thanks,
> Khushboo
>

Re: [pgadmin4][patch] Initial patch to decouple from ACI Tree

[Joao De Almeida Pereira]

Hello Ashesh,

1. In TreeNode, we're keepging the reference of DOMElement, do we really
>> need it?
>
> As of right now, our Tree abstraction acts as an adapter to the aciTree
>> library. The aciTree library needs the domElement for most of its functions
>> (setInode, unload, etc). Thus this is the easiest way to introduce our
>> abstraction and keep the functionality as before - at least until we decide
>> that whether we want to switch out the library or not.
>
> I understand that. But - I've not seen any reference of domElement the
> code yet, hence - pointed that out.

If you look at the function: reload, unload you will see that domNode is
used to communicate with the ACITree
​

> 2. Are you expecting the tree class to be a singleton class
>
> Since this tree is referenced throughout the codebase, considering it to
>> be a singleton seems like the most appropriate pattern for this usecase. It
>> is very much the same way how we create a single instance of the aciTree
>> library and use that throughout the codebase. Moreover, it opens up
>> opportunities to improve performance, for example caching lockups of nodes.
>> I’m not a fan of singletons myself, but I feel like we’re simply keeping
>> the architecture the same in the instance.
>
> Yeah - I don't see any usage of tree object from anywhere.
> And, we're already creating new object in browser.js (and, not utitlizing
> that instance anywhere.)


You are right, we do not need to export tree as a singleton for now. The
line that exports the variable tree can be remove when applying the patch
number 2.
​

I think we addressed all the concern raised about this patch. Does this
mean that the patch is going to get committed?

We are looking forward to continue the development on this track of work,
but as you know, we are blocked until it gets committed into master. We
hope that this gets unblocked soon because this is a very big track of work
and the longer it is blocked, the longer it will take to get finalized.

Thanks
Joao


On Fri, May 11, 2018 at 2:32 AM Ashesh Vashi 
wrote:

> On Thu, May 10, 2018 at 8:08 PM, Anthony Emengo 
> wrote:
>
>> 1. In TreeNode, we're keepging the reference of DOMElement, do we really
>>> need it?
>>
>> As of right now, our Tree abstraction acts as an adapter to the aciTree
>> library. The aciTree library needs the domElement for most of its functions
>> (setInode, unload, etc). Thus this is the easiest way to introduce our
>> abstraction and keep the functionality as before - at least until we decide
>> that whether we want to switch out the library or not.
>>
> I understand that. But - I've not seen any reference of domElement the
> code yet, hence - pointed that out.
>
>> 2. Are you expecting the tree class to be a singleton class
>>
>> Since this tree is referenced throughout the codebase, considering it to
>> be a singleton seems like the most appropriate pattern for this usecase. It
>> is very much the same way how we create a single instance of the aciTree
>> library and use that throughout the codebase. Moreover, it opens up
>> opportunities to improve performance, for example caching lockups of nodes.
>> I’m not a fan of singletons myself, but I feel like we’re simply keeping
>> the architecture the same in the instance.
>>
> Yeah - I don't see any usage of tree object from anywhere.
> And, we're already creating new object in browser.js (and, not utitlizing
> that instance anywhere.)
>
> -- Thanks, Ashesh
>
>> ​
>>
>> Sincerely,
>>
>> Anthony and Victoria
>>
>
>

Re: [pgadmin4][patch] Initial patch to decouple from ACI Tree

[Joao De Almeida Pereira]

You've lost us with the last reply. We'd love to know what we'd have to do
to get this patch committed. You mention that "some things are missing at
some places" and that "there are missing bits". Please note that this is
not a complete solution that we've offered so far. But only one step in a
grander effort to effect a more cleaner, more maintainable, more testable,
code base.

Thanks
Joao and Anthony.

On Mon, Apr 30, 2018 at 11:45 AM Ashesh Vashi <ashesh.va...@enterprisedb.com>
wrote:

> On Mon, Apr 30, 2018 at 9:07 PM, Dave Page <dp...@pgadmin.org> wrote:
>
>>
>>
>> On Mon, Apr 30, 2018 at 4:33 PM, Ashesh Vashi <
>> ashesh.va...@enterprisedb.com> wrote:
>>
>>>
>>> On Mon, Apr 30, 2018 at 8:58 PM, Anthony Emengo <aeme...@pivotal.io>
>>> wrote:
>>>
>>>> I was expecting a separate layer between the tree implementation, and
>>>>> aciTree adaptor.
>>>>> Please find the patch for the example.
>>>>> It will separate the two layers, and easy to replace with the new
>>>>> implementation in future.
>>>>
>>>>
>>>> In general, we want defer the separation of the layers for now. Even
>>>> though we might assume that this is the direction we want to go in. It's
>>>> simply too early to be making such an architectural leap. For right now, we
>>>> just know that we need the decoupling, but don't know what if we'd need the
>>>> 2 layers *as implemented*. The principle we're adhering to here is the
>>>> Last Responsible Moment principle, which states that you only make the
>>>> changes that you feel is necessary for the given problems you're facing:
>>>> https://blog.codinghorror.com/the-last-responsible-moment/
>>>>
>>>> I would not like to see that changes in this patch.
>>>>> I would like us to come up with the actual design about the hot
>>>>> pluggability before going in this direction.
>>>>
>>>>
>>>> In our point of view these 2 changes are not related. One thing is the
>>>> internal code organization of the application, other thing is allowing
>>>> third party to drop code in the application and it just work. These 2
>>>> should be talked separately, but the hot pluggability is not something that
>>>> will be address by this work we are doing right now.
>>>>
>>> Neither - it should be part of this change.
>>> It should be addressed separately, and discussed.
>>>
>>
>> I agree. As long as this work doesn't make the pluggability problem
>> worse, that problem should be considered separately.
>>
>> So given Anthony's comments, are you happy with this patch?
>>
> I liked the design so far.
> But - as Khushboo mentioned ealier - it is missing at some places.
> I had to read through the code to understand the execution flow for some.
>
> And, there is still a lot missing bits, and pieces to consider for commit
> in the repo.
>
> -- Thanks, Ashesh
>
>>
>>
>>>
>>> -- Thanks, Ashesh
>>>
>>>>
>>>> Anthony && Joao
>>>>
>>>> On Mon, Apr 30, 2018 at 11:03 AM, Ashesh Vashi <
>>>> ashesh.va...@enterprisedb.com> wrote:
>>>>
>>>>>
>>>>>
>>>>>
>>>>> On Mon, Apr 30, 2018 at 8:30 PM, Ashesh Vashi <
>>>>> ashesh.va...@enterprisedb.com> wrote:
>>>>>
>>>>>> On Sat, Apr 28, 2018 at 3:55 AM, Joao De Almeida Pereira <
>>>>>> jdealmeidapere...@pivotal.io> wrote:
>>>>>>
>>>>>>> Hi Hackers,
>>>>>>> As you are aware we kept on working on the patch, so we are
>>>>>>> attaching to this email a new version of the patch.
>>>>>>> This new version contains all the changes in the previous one plus
>>>>>>> more extractions of functions and refactoring of code.
>>>>>>>
>>>>>>> The objective of this patch is to create a separation between
>>>>>>> pgAdmin and the ACI Tree. We are doing this because we realized that at
>>>>>>> this point in time we have the ACI Tree all over the code of pgAdmin. I
>>>>>>> found a very interesting article that really talks about this:
>>>>>>> https://medium.freecodecamp.org/code-dependencies-are-the-devil-35ed28b556d
>>>>>>>
>>

Re: [pgAdmin4][Patch]: RM 3284 - F5 key not working consistently

[Joao De Almeida Pereira]

Hi Khushboo,

I did some changes on your patch:
 0001 - Your original patch
 0002 - Convert keyboard.js to ES6
 0003 - Refactoring of the keyboard.js file(some one letter variables and
other code)



Thanks
Joao

On Thu, Apr 26, 2018 at 5:34 AM Khushboo Vashi <
khushboo.va...@enterprisedb.com> wrote:

> Hi,
>
> Please find the attached patch to fix the RM #3284 : F5 key not working
> consistently.
>
> - Added the configurable keyboard shortcut (default F5) to refresh the
> browser tree nodes.
>
>
> Thanks,
> Khushboo
>


0001-RM_3284_v1.patch
Description: Binary data


0002-Convert-keyboard.js-to-ES6.patch
Description: Binary data


0003-Refactoring-of-keyboard.js.patch
Description: Binary data

Re: [pgAdmin4][Patch] Feature #1447 SSH Tunnel

[Joao De Almeida Pereira]

Hi Akshay,

Some suggestions:
- browser/server_groups/servers/__init__py
  This file could have been split into separate functionalities. There is a
chunk of changes for connect, why not move that out? Same thing for create.
Do we really need to have full integrationy tests that do a HTTP request
and connect to a real database to do make sure the functionalities are
working? If we isolate these into their own actions we can more easily get
more coverage on the code with tests that would be much faster and
directed. The big advantage of this is that by reading the tests we can
understand what the functions do. Self documenting code.

- utils/driver/psycopg2.py same comments has above

- browser/server_groups/servers/static/js/server.js:
  - The patterm of using `m` for `model`? is a bad pattern, so why not
change it?
  - We could extract the model creation from this file. This will allow us
to add some tests around disabled methods that are a bit everywhere
  - We could also convert this file to ES6

- utils/driver/psycopg2/server_manager.py
  - Do we have Unit Tests around this?
  - Maybe this SSH part could be isolated into it's own class, as it is not
100% related to the class in question. We need to use it but is is not part
of the ServerManager domain

- JS template. Eventually I would like to see if completely removed, and
the information that we are generating using the template can be passed to
the frontend via a Ajax call as an example( Do not think this is the time
to do it.)

- start_running_query.py
 - we could enrich the tests of this functionality


And example of naming is for example on psycopg2/connection.py

mgr = self.manager

How much to we win by having this variable name versus manager =
self.manager or even using the self.manager?


This is not for you in specific, but for @hackers in general:
The book https://www.amazon.com/dp/0132350882/
<https://www.amazon.com/dp/0132350882/?tag=stackoverflow17-20> is a pretty
nice book that gives you an introduction to clean code, that is self
documenting and that is much easily maintained.

Thanks
Joao

On Thu, Apr 26, 2018 at 3:44 AM Akshay Joshi <akshay.jo...@enterprisedb.com>
wrote:

> Hi Hackers,
>
> Attached is the updated patch which includes documentation of the feature
> and also updated screenshots of server dialog with new "SSH Tunnel" tab.
>
> On Wed, Apr 25, 2018 at 11:45 AM, Akshay Joshi <
> akshay.jo...@enterprisedb.com> wrote:
>
>> Hi Joao
>>
>> On Tue, Apr 24, 2018 at 10:04 PM, Joao De Almeida Pereira <
>> jdealmeidapere...@pivotal.io> wrote:
>>
>>> Hi Akshay,
>>>
>>> After looking through the patch we found some one letter variable names
>>> and this is a regression on what we have been trying to accomplish in the
>>> last year.
>>>
>>> An objective that we have for pgAdmin source code is to increase the
>>> testability of it and make it more readable. If we keep on adding one
>>> letter variables and if we continue adding code to already convoluted
>>> source files it is going to be very hard to achieve this objective.
>>>
>>
>>   At my level I have tried not to give one letter variable names.
>> Are you talking about the variable "m" in server.js file which
>> represents the Model? If yes then I have followed the code written for
>> whole schema and I thought we have to maintain the consistency, so use that
>> as it is. Apart from that I haven't seen any other one letter variable,
>> please correct me so that I'll rename it.
>>
>>>
>>> Our recommendations for this change are:
>>> - Name the variables with comprehensive names
>>>
>>   Can you please suggest from the patch.
>>
>>
>>> - Extract functions where we can and try to wrap some tests around them
>>> (ex: the javascript disabled functions)
>>>
>>
>> I have tried to do that too, if you can see the "server/__init__.py"
>> file I have created "*get_response_for_password" function to remove
>> redundant code. Based on the condition it will return the json response.*
>>
>>
>>> - We really need to find a better pattern than templated Javascript to
>>> pass information from the backend to the frontend
>>>
>> - When changing a piece of code, if we see code that is confusing or that
>>> is hard to read, we should refactor instead of adding to the pattern.
>>>
>>
>> Please elaborate more with respect to my patch, which part of code
>> should required modification?
>>
>>>
>>> Thanks
>>> Victoria & Joao
>>>
>>>
>>> On Tue, Apr 24, 2018 at 10:13 AM Akshay Joshi &

[pgadmin4][patch] Old code failing CI

[Joao De Almeida Pereira]

Hi Hackers,
Attached you can find a patch of some leftover code that we didn't realize
was still there.

Thanks
Joao


old-code.patch
Description: Binary data

Re: [pgAdmin4][RM#3155] Allow user to lock the Layout

[Joao De Almeida Pereira]

Hi,
@Murtuza: We didn't notice the issue, can you please advise on what need to
change to make it work? The only change we did was to make one test pass.

@Hackers: In our point of view it is never good to fork a library. But if
he really have to do it, then we should fork it in Github, make our code
accessible to other people, and we should add it as a dependency on
package.json


Thanks
Anthony & Joao


On Wed, Apr 25, 2018 at 7:14 AM Dave Page <dp...@pgadmin.org> wrote:

> On Wed, Apr 25, 2018 at 12:13 PM, Murtuza Zabuawala <
> murtuza.zabuaw...@enterprisedb.com> wrote:
>
>> Hi Dave,
>>
>> On Wed, Apr 25, 2018 at 3:36 PM, Dave Page <dp...@pgadmin.org> wrote:
>>
>>> All,
>>>
>>> We just had a brief discussion in our EDB sprint planning meeting about
>>> this. There is a non-zero chance that we're going to have to fork wcDocker
>>> in the near future, in order to update it to work with jQuery 3. If we do
>>> that, then it may be significantly easier to fix this issue in that fork
>>> (perhaps by adding a single lockLayout(bool) function, rather than trying
>>> to do so from pgAdmin.
>>>
>>> I think (unless Murtuza believes that won't help), that we're better off
>>> holding on this for now until we know if we've had to do that.
>>>
>>
>> ​I don't have any objection forking the code and adding the flag to lock
>> the panel,  But I'm certain that
>> we will use the same inbuilt method *panel.moveable(false)* which we
>> have used right now in the patch to prevent a panel from floating and will
>> face the same issue again which Akshay mentioned in his last email.
>>
>> Let me know if you want me to attach latest patch onto the ticket for
>> future reference and update the ticket accordingly​.
>>
>
> That's probably a good idea - thanks.
>
>
>>
>>
>>> On Wed, Apr 25, 2018 at 10:23 AM, Murtuza Zabuawala <
>>> murtuza.zabuaw...@enterprisedb.com> wrote:
>>>
>>>> Hi Akshay,
>>>>
>>>>
>>>> On Wed, Apr 25, 2018 at 2:37 PM, Akshay Joshi <
>>>> akshay.jo...@enterprisedb.com> wrote:
>>>>
>>>>> Hi Joao/Murtuza
>>>>>
>>>>> It break's the functionality, I am able to move "Data output",
>>>>> "Explain" etc.. panel of Query Tool, even if "Lock layout?" is set to 
>>>>> True.
>>>>>
>>>>
>>>> ​It's working properly in v5 patch, Something went wrong while
>>>> refactoring.​
>>>>
>>>> ​
>>>>
>>>> Apart from above I have found more issue. Below are the steps to
>>>>> reproduce:
>>>>>
>>>>>- Set the "Lock layout?" flag to False.
>>>>>- Move out Dashboard panel.
>>>>>- Set the "Lock layout?" flag to True.
>>>>>    - Close the Dashboard panel, as layout is locked and empty
>>>>>Dashboard panel is still visible. (Refer attached screenshot)
>>>>>
>>>>> ​That's because we have set the Panel moveable property to False, they
>>>> won't auto resize, As discussed earlier if user drag any panel out of panel
>>>> group it gets render in seprate wcFrame. I think that needs to be taken
>>>> care by user before they decide to lock the layout, We can not expilcitly
>>>> set panel's closeable property to False when layout is locked, If we do so
>>>> user will not be able to close any Query tool, Debugger panels.​
>>>>  ​
>>>>
>>>>
>>>> On Tue, Apr 24, 2018 at 9:11 PM, Joao De Almeida Pereira <
>>>>> jdealmeidapere...@pivotal.io> wrote:
>>>>>
>>>>>> haha,
>>>>>> Just joking, now we have a good version that passes tests and all.
>>>>>>
>>>>>> We found out that a test was failing in the patch version 5:
>>>>>>
>>>>>> HeadlessChrome 0.0.0 (Linux 0.0.0) Panel when we create a panel and user 
>>>>>> created panel without defining isMoveable then it should be moveable it 
>>>>>> should call moveable method with true as argument FAILED
>>>>>> Expected false to be true.
>>>>>> at UserContext. 
>>>>>> (regression/javascript/browser/panel_spec.js:12886:38)
>>>>>>
>>>>>> ​
>>>>>> To solve this problem we decided to 

[pgadmin4][patch] [GreenPlum] Partitioned table icon is the same as non partitioned tables

[Joao De Almeida Pereira]

Hi Hackers,
Attached you can find 2 patches that correct the Redmine: #3308
 A
Patches:
0001 - In this patch we update the SQL to retrieve the is_partitioned flag
from the database
0002 - Extract and refactor the icon css class retrieval. It was scattered
around the code and we moved it into it's own small class

We also realize that the class was present in some places in the javascript
world. What it is strange is that we only override the icon with the
partition one after a successful truncation or successful reset of
statistics.
Why was this behavior added? Can we remove that?


Thanks
Victoria & Joao
diff --git a/web/pgadmin/browser/server_groups/servers/databases/schemas/tables/templates/table/sql/gpdb_5.0_plus/nodes.sql b/web/pgadmin/browser/server_groups/servers/databases/schemas/tables/templates/table/sql/gpdb_5.0_plus/nodes.sql
index 3e877bfc..8c1e5878 100644
--- a/web/pgadmin/browser/server_groups/servers/databases/schemas/tables/templates/table/sql/gpdb_5.0_plus/nodes.sql
+++ b/web/pgadmin/browser/server_groups/servers/databases/schemas/tables/templates/table/sql/gpdb_5.0_plus/nodes.sql
@@ -1,6 +1,7 @@
 SELECT rel.oid, rel.relname AS name,
 (SELECT count(*) FROM pg_trigger WHERE tgrelid=rel.oid) AS triggercount,
-(SELECT count(*) FROM pg_trigger WHERE tgrelid=rel.oid AND tgenabled = 'O') AS has_enable_triggers
+(SELECT count(*) FROM pg_trigger WHERE tgrelid=rel.oid AND tgenabled = 'O') AS has_enable_triggers,
+(CASE WHEN (SELECT count(*) from pg_partition where parrelid = rel.oid) > 0 THEN true ELSE false END) AS is_partitioned
 FROM pg_class rel
 WHERE rel.relkind IN ('r','s','t') AND rel.relnamespace = {{ scid }}::oid
   AND rel.relname NOT IN (SELECT partitiontablename FROM pg_partitions)
-- 
2.17.0

diff --git a/web/pgadmin/browser/server_groups/servers/databases/schemas/tables/__init__.py b/web/pgadmin/browser/server_groups/servers/databases/schemas/tables/__init__.py
index d49ca20e..a4c229a1 100644
--- a/web/pgadmin/browser/server_groups/servers/databases/schemas/tables/__init__.py
+++ b/web/pgadmin/browser/server_groups/servers/databases/schemas/tables/__init__.py
@@ -303,21 +303,17 @@ class TableView(BaseTableView, DataTypeReader, VacuumSettings):
 if len(rset['rows']) == 0:
 return gone(gettext("Could not find the table."))
 
-if 'is_partitioned' in rset['rows'][0] and \
-rset['rows'][0]['is_partitioned']:
-icon = "icon-partition"
-else:
-icon = "icon-table"
+table_information = rset['rows'][0]
+icon = self.get_icon_css_class(table_information)
 
 res = self.blueprint.generate_browser_node(
-rset['rows'][0]['oid'],
+table_information['oid'],
 scid,
-rset['rows'][0]['name'],
+table_information['name'],
 icon=icon,
-tigger_count=rset['rows'][0]['triggercount'],
-has_enable_triggers=rset['rows'][0]['has_enable_triggers'],
-is_partitioned=rset['rows'][0]['is_partitioned'] if
-'is_partitioned' in rset['rows'][0] else False
+tigger_count=table_information['triggercount'],
+has_enable_triggers=table_information['has_enable_triggers'],
+is_partitioned=self.is_table_partitioned(table_information)
 )
 
 return make_json_response(
@@ -350,10 +346,7 @@ class TableView(BaseTableView, DataTypeReader, VacuumSettings):
 return internal_server_error(errormsg=rset)
 
 for row in rset['rows']:
-if 'is_partitioned' in row and row['is_partitioned']:
-icon = "icon-partition"
-else:
-icon = "icon-table"
+icon = self.get_icon_css_class(row)
 
 res.append(
 self.blueprint.generate_browser_node(
@@ -363,8 +356,7 @@ class TableView(BaseTableView, DataTypeReader, VacuumSettings):
 icon=icon,
 tigger_count=row['triggercount'],
 has_enable_triggers=row['has_enable_triggers'],
-is_partitioned=row['is_partitioned'] if
-'is_partitioned' in row else False,
+is_partitioned=self.is_table_partitioned(row),
 rows_cnt=0
 ))
 
@@ -968,13 +960,11 @@ class TableView(BaseTableView, DataTypeReader, VacuumSettings):
 
 try:
 partitions_sql = ''
-partitioned = False
-if 'is_partitioned' in data and data['is_partitioned']:
+if self.is_table_partitioned(data):
 data['relkind'] = 'p'
 # create partition scheme
 data['partition_scheme'] = self.get_partition_scheme(data)
 partitions_sql = self.get_partitions_sql(data)
-partitioned = True
 
 SQL = render_template(
 

Re: [pgadmin4][patch] Initial patch to decouple from ACI Tree

[Joao De Almeida Pereira]

Hi Hackers,

Can someone review and merge this patch?

Thanks
Joao

On Wed, Apr 18, 2018 at 10:23 AM Joao De Almeida Pereira <
jdealmeidapere...@pivotal.io> wrote:

> Hi Hackers,
> Any other comment about this patch?
>
> Thanks
> Joao
>
> On Tue, Apr 10, 2018 at 12:00 PM Joao De Almeida Pereira <
> jdealmeidapere...@pivotal.io> wrote:
>
>> Hello Khushboo
>>
>> On Mon, Apr 9, 2018 at 1:59 AM Khushboo Vashi <
>> khushboo.va...@enterprisedb.com> wrote:
>>
>>> Hi Joao,
>>>
>>> I have reviewed your patch and have some suggestions.
>>>
>>> On Sat, Apr 7, 2018 at 12:43 AM, Joao De Almeida Pereira <
>>> jdealmeidapere...@pivotal.io> wrote:
>>>
>>>> Hello Murtuza/Dave,
>>>> Yes now the extracted functions are spread into different files. The
>>>> intent would be to make the files as small as possible, and also to group
>>>> and name them in a way that would be easy to understand what each file is
>>>> doing without the need of opening it.
>>>> As a example:
>>>> static/js/backup will contain all the backup related functionality
>>>> inside of this folder we can see the file:
>>>>
>>> menu_utils.js At this moment in time we decided to group all the
>>>> functions that are related to the menu, but we can split that also if we
>>>> believe it is easier to see.
>>>>
>>> It's really very good to see the separated code for backup module. As we
>>> have done for backup, we would like do it for other PG utilities like
>>> restore, maintenance etc.
>>> Considering this, we should separate the code in a way that some of the
>>> common functionalities can be used for other modules  like menu (as you
>>> have mentioned above), dialogue factory etc.
>>> Also, I think these functionalities should be in their respective static
>>> folder instead of pgadmin/static.
>>>
>>
>> About the location of the files. The move of the files to
>> pgadmin/static/js was made on purpose in order to clearly separate
>> Javascript from python code.
>> The rational behind it was
>> - Create a clear separation between the backend and frontend
>> - Having Javascript code concentrated in a single place, hopefully, will
>> encourage to developers to look for a functionality, that is already
>> implemented in another modules, because they are right there. (When we
>> started this journey we realized that the 'nodes' have a big groups of code
>> that could be shared, but because the Javascript is spread everywhere it is
>> much harder to look for it)
>>
>>
>> There are some drawbacks of this separation:
>> - When creating a new module we will need to put the javascript in a
>> separate location from the backend code
>>
>>
>>>
>>>
>>>> static/js/datagrid folder contains all the datagrid related
>>>> functionality
>>>>
>>> Same as backup module,  this should be in it's respective static/js
>>> folder.
>>>
>>>> Inside of the folder we can see the files:
>>>> get_panel_title.js is responsible for retrieving the name of the panel
>>>> show_data.js is responsible for showing the datagrid
>>>> show_query_tool.js is responsible for showing the query tool
>>>>
>>>> Does this structure make sense?
>>>> Can you give an example of a comment that you think is missing and that
>>>> could help?
>>>>
>>>> As a personal note, unless the algorithm is very obscure or very
>>>> complicated, I believe that if the code needs comments it is a signal that
>>>> something needs to change in terms of naming, structure of the part in
>>>> question. This being said, I am open to add some comments that might help
>>>> people.
>>>>
>>> You are right, with the help of naming convention and structure of the
>>> code, any one can get the idea about the code. But it is very useful to
>>> understand the code
>>> very easily with the proper comments especially when there are multiple
>>> developers working on a single project.
>>>
>>> I found some of the places where it would be great to have comments.
>>>
>>> - treeMenu: new tree.Tree()  in a browser.js
>>> - tree.js  (especially Tree class)
>>>
>> About the comment point I need a more clear understanding on what kind of
>> comments you are looking for. Because when you read the

Re: [pgAdmin4][Patch] Feature #1447 SSH Tunnel

[Joao De Almeida Pereira]

Hi Akshay,

After looking through the patch we found some one letter variable names and
this is a regression on what we have been trying to accomplish in the last
year.

An objective that we have for pgAdmin source code is to increase the
testability of it and make it more readable. If we keep on adding one
letter variables and if we continue adding code to already convoluted
source files it is going to be very hard to achieve this objective.

Our recommendations for this change are:
- Name the variables with comprehensive names
- Extract functions where we can and try to wrap some tests around them
(ex: the javascript disabled functions)
- We really need to find a better pattern than templated Javascript to pass
information from the backend to the frontend
- When changing a piece of code, if we see code that is confusing or that
is hard to read, we should refactor instead of adding to the pattern.

Thanks
Victoria & Joao


On Tue, Apr 24, 2018 at 10:13 AM Akshay Joshi 
wrote:

> Hi Hackers
>
> As per suggestion by Dave, I have moved "Advanced" tab at the last for
> Server dialog. Attached is the modified patch.
>
> On Mon, Apr 23, 2018 at 7:32 PM, Anthony Emengo 
> wrote:
>
>> For what it is worth, I manually verified that the feature worked, as
>> well as looked through the code.
>>
>> I'd like to see end-to-end testing for regression sake, but it's hard to
>> so at this moment.
>>
>> - Anthony and Joao.
>>
>> On Mon, Apr 23, 2018 at 5:09 AM, Akshay Joshi <
>> akshay.jo...@enterprisedb.com> wrote:
>>
>>>
>>>
>>> On Mon, Apr 23, 2018 at 1:30 PM, Dave Page  wrote:
>>>
 Hi

 On Thu, Apr 19, 2018 at 6:56 PM, Anthony Emengo 
 wrote:

> Hey Akshay
>
> This patch passed our test pipelines.
>

 Did you test the feature and//or review the code and tests? Passing the
 tests is great, *if* the whole feature is covered (and the nature of this
 patch will make that quite difficult, maybe impossible to do without
 external infrastructure and config).

>>>
>>> Agreed, it's been difficult to write test case to test the complete
>>> feature.
>>>


>
> Anthony and Victoria
>
> On Thu, Apr 19, 2018 at 1:48 AM, Akshay Joshi <
> akshay.jo...@enterprisedb.com> wrote:
>
>> Hi Hackers
>>
>> I have implemented the SSH Tunnel support using
>> https://pypi.org/project/sshtunnel/ python package. Added "SSH
>> Tunnel" Tab in server dialog. This implementation supports user name
>> /password and private/public key combination with Passphrase to crate SSH
>> Tunnel. I have added regression test case to add server using SSH Tunnel
>> options.
>>
>> The given python package(https://pypi.org/project/sshtunnel/) support
>> Python version *2.7, 3.4+*.
>> It uses Paramiko (Python implementation of SSHv2 protocol) which
>> actually drops support for Python 2.6. So I have added
>> *SUPPORT_SSH_TUNNEL* parameter in config.py which checks the python
>> version and set the flag accordingly. In case of Python 2.6, 3.0, 3.1, 
>> 3.2
>> and 3.3 control on the "SSH Tunnel" tab of server dialog will be 
>> disabled.
>>
>> Please review it, and if looks good please commit the code.
>>
>> --
>> *Akshay Joshi*
>>
>> *Sr. Software Architect *
>>
>>
>>
>> *Phone: +91 20-3058-9517 <+91%2020%203058%209517>Mobile: +91
>> 976-788-8246 <+91%2097678%2088246>*
>>
>
>


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

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

>>>
>>>
>>>
>>> --
>>> *Akshay Joshi*
>>>
>>> *Sr. Software Architect *
>>>
>>>
>>>
>>> *Phone: +91 20-3058-9517 <+91%2020%203058%209517>Mobile: +91
>>> 976-788-8246 <+91%2097678%2088246>*
>>>
>>
>>
>
>
> --
> *Akshay Joshi*
>
> *Sr. Software Architect *
>
>
>
> *Phone: +91 20-3058-9517 <+91%2020%203058%209517>Mobile: +91 976-788-8246
> <+91%2097678%2088246>*
>

Re: [pgadmin4][patch] Initial patch to decouple from ACI Tree

[Joao De Almeida Pereira]

Hi Hackers,
Any other comment about this patch?

Thanks
Joao

On Tue, Apr 10, 2018 at 12:00 PM Joao De Almeida Pereira <
jdealmeidapere...@pivotal.io> wrote:

> Hello Khushboo
>
> On Mon, Apr 9, 2018 at 1:59 AM Khushboo Vashi <
> khushboo.va...@enterprisedb.com> wrote:
>
>> Hi Joao,
>>
>> I have reviewed your patch and have some suggestions.
>>
>> On Sat, Apr 7, 2018 at 12:43 AM, Joao De Almeida Pereira <
>> jdealmeidapere...@pivotal.io> wrote:
>>
>>> Hello Murtuza/Dave,
>>> Yes now the extracted functions are spread into different files. The
>>> intent would be to make the files as small as possible, and also to group
>>> and name them in a way that would be easy to understand what each file is
>>> doing without the need of opening it.
>>> As a example:
>>> static/js/backup will contain all the backup related functionality
>>> inside of this folder we can see the file:
>>>
>> menu_utils.js At this moment in time we decided to group all the
>>> functions that are related to the menu, but we can split that also if we
>>> believe it is easier to see.
>>>
>> It's really very good to see the separated code for backup module. As we
>> have done for backup, we would like do it for other PG utilities like
>> restore, maintenance etc.
>> Considering this, we should separate the code in a way that some of the
>> common functionalities can be used for other modules  like menu (as you
>> have mentioned above), dialogue factory etc.
>> Also, I think these functionalities should be in their respective static
>> folder instead of pgadmin/static.
>>
>
> About the location of the files. The move of the files to
> pgadmin/static/js was made on purpose in order to clearly separate
> Javascript from python code.
> The rational behind it was
> - Create a clear separation between the backend and frontend
> - Having Javascript code concentrated in a single place, hopefully, will
> encourage to developers to look for a functionality, that is already
> implemented in another modules, because they are right there. (When we
> started this journey we realized that the 'nodes' have a big groups of code
> that could be shared, but because the Javascript is spread everywhere it is
> much harder to look for it)
>
>
> There are some drawbacks of this separation:
> - When creating a new module we will need to put the javascript in a
> separate location from the backend code
>
>
>>
>>
>>> static/js/datagrid folder contains all the datagrid related
>>> functionality
>>>
>> Same as backup module,  this should be in it's respective static/js
>> folder.
>>
>>> Inside of the folder we can see the files:
>>> get_panel_title.js is responsible for retrieving the name of the panel
>>> show_data.js is responsible for showing the datagrid
>>> show_query_tool.js is responsible for showing the query tool
>>>
>>> Does this structure make sense?
>>> Can you give an example of a comment that you think is missing and that
>>> could help?
>>>
>>> As a personal note, unless the algorithm is very obscure or very
>>> complicated, I believe that if the code needs comments it is a signal that
>>> something needs to change in terms of naming, structure of the part in
>>> question. This being said, I am open to add some comments that might help
>>> people.
>>>
>> You are right, with the help of naming convention and structure of the
>> code, any one can get the idea about the code. But it is very useful to
>> understand the code
>> very easily with the proper comments especially when there are multiple
>> developers working on a single project.
>>
>> I found some of the places where it would be great to have comments.
>>
>> - treeMenu: new tree.Tree()  in a browser.js
>> - tree.js  (especially Tree class)
>>
> About the comment point I need a more clear understanding on what kind of
> comments you are looking for. Because when you read the function names you
> understand the intent, what they are doing. The parameters also explain
> what you need to pass into them.
>
> If what you are looking for in these comments is the reasoning being the
> change itself, then that should be present in the commit message. Specially
> because this is going to be a very big patch with a very big number of
> changes.
>
>>
>> Thanks
>>> Joao
>>> ​
>>>
>>> Thanks,
>> Khushboo
>>
>>>
>>> On Fri, Apr 6, 2018 at 4:48 AM Murtuza Z

Re: pgAdmin 3.0 builds

[Joao De Almeida Pereira]

Hi Hackers,
Good job getting this release out. It was hard work, but you were able to
pull it off.

Looking back at the last 3 week I have a feeling that we should discuss if
there is any way to change the release process in order to make it less
painful, more automated and be able to release in shorter cycles.


Good Job
Joao

On Thu, Apr 12, 2018 at 12:02 PM Fahar Abbas <fahar.ab...@enterprisedb.com>
wrote:

> Hi Dave,
>
> I have finished one round of sanity testing for pgAdmin4 3.0 and no issue
> found during verification and it's ready for the release.
>
> Kind Regards,
>
> On Thu, Apr 12, 2018 at 7:14 PM, Dave Page <dp...@pgadmin.org> wrote:
>
>> Hi
>>
>> On Thu, Apr 12, 2018 at 2:29 PM, Joao De Almeida Pereira <
>> jdealmeidapere...@pivotal.io> wrote:
>>
>>> Hi Dave,
>>> Did you remove the folder with the builds?
>>>
>>
>> Yes, when I re-built everything. It's now at:
>> https://developer.pgadmin.org/~dpage/v3.0-2/
>>
>>
>>> What is the git SHA from which the release is going to be created from?
>>>
>>
>> 79edf40141b5886634ef71fb5bf14ec076e2adaf
>>
>> I'll push the REL-3_0 tag, once Fahar tells me everything looks good
>> (expected to be later today, formal release tomorrow AM).
>>
>>
>>>
>>> Thanks
>>> Joao
>>>
>>> On Thu, Apr 12, 2018 at 4:08 AM Dave Page <dp...@pgadmin.org> wrote:
>>>
>>>> Hi
>>>>
>>>> On Wed, Apr 11, 2018 at 6:08 PM, Christoph Berg <m...@debian.org>
>>>> wrote:
>>>>
>>>>> Re: Dave Page 2018-04-11 <
>>>>> ca+ocxoxvshpmyz0rxaq+e5+3gxohw0oitv-ha4_ssn+23a0...@mail.gmail.com>
>>>>> > New builds are at https://developer.pgadmin.org/~dpage/v3.0-2/.
>>>>> >
>>>>> > These fix the Windows crash that Fahar found (which happened if the
>>>>> user
>>>>> > had a space in their usename), and fix password
>>>>> encryption/decryption under
>>>>> > Python 3.
>>>>> >
>>>>> > Apologies for the inconvenience again. I still hope to release
>>>>> tomorrow,
>>>>> > but let me know if that's not enough time and I can do Friday
>>>>> instead.
>>>>>
>>>>> I won't be able to create the Debian packages this week because I'm at
>>>>> pgconf.de.
>>>>>
>>>>
>>>> No problem.
>>>>
>>>>
>>>>>
>>>>> > >> - This release removes dependencies on QtWebKit and/or
>>>>> QtWebEngine, and
>>>>> > >> uses the users default browser instead.
>>>>>
>>>>> This would need more testing anyway.
>>>>>
>>>>
>>>> Should be pretty straightforward - in RPM terms it's a one-line change
>>>> to remove a "Requires" line from the spec file. Updates to the other
>>>> dependency packages will almost certainly take a lot more time
>>>> unfortunately.
>>>>
>>>>
>>>>>
>>>>> > >> - Gnome 3.26 or above will require the TopIcons Plus extension to
>>>>> be
>>>>> > >> installed (or an equivalent). Ubuntu already seems to have this,
>>>>> per my
>>>>> > >> testing. Fedora does not.
>>>>>
>>>>> Hmm. This seems to be very desktop specific. What if the user isn't
>>>>> using Gnome at all?
>>>>>
>>>>
>>>> From my research, Gnome seems to be the only window manager where this
>>>> is an issue (it seems to be quite a controversial change). We will be
>>>> working on a better, more permanent solution in the future, however that's
>>>> a lot of effort, and not something we could hold up 3.0 for.
>>>>
>>>> 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
>>
>
>
>
> --
> Fahar Abbas
> QMG
> EnterpriseDB Corporation
> Phone Office: +92-51-835-8874
> Phone Direct: +92-51-8466803 <+92%2051%208466803>
> Mobile: +92-333-5409707 <+92%20333%205409707>
> Skype ID: syed.fahar.abbas
> Website: www.enterprisedb.com
>

Re: pgAdmin 3.0 builds

[Joao De Almeida Pereira]

Hi Dave,
Did you remove the folder with the builds?
What is the git SHA from which the release is going to be created from?

Thanks
Joao

On Thu, Apr 12, 2018 at 4:08 AM Dave Page  wrote:

> Hi
>
> On Wed, Apr 11, 2018 at 6:08 PM, Christoph Berg  wrote:
>
>> Re: Dave Page 2018-04-11 <
>> ca+ocxoxvshpmyz0rxaq+e5+3gxohw0oitv-ha4_ssn+23a0...@mail.gmail.com>
>> > New builds are at https://developer.pgadmin.org/~dpage/v3.0-2/.
>> >
>> > These fix the Windows crash that Fahar found (which happened if the user
>> > had a space in their usename), and fix password encryption/decryption
>> under
>> > Python 3.
>> >
>> > Apologies for the inconvenience again. I still hope to release tomorrow,
>> > but let me know if that's not enough time and I can do Friday instead.
>>
>> I won't be able to create the Debian packages this week because I'm at
>> pgconf.de.
>>
>
> No problem.
>
>
>>
>> > >> - This release removes dependencies on QtWebKit and/or QtWebEngine,
>> and
>> > >> uses the users default browser instead.
>>
>> This would need more testing anyway.
>>
>
> Should be pretty straightforward - in RPM terms it's a one-line change to
> remove a "Requires" line from the spec file. Updates to the other
> dependency packages will almost certainly take a lot more time
> unfortunately.
>
>
>>
>> > >> - Gnome 3.26 or above will require the TopIcons Plus extension to be
>> > >> installed (or an equivalent). Ubuntu already seems to have this, per
>> my
>> > >> testing. Fedora does not.
>>
>> Hmm. This seems to be very desktop specific. What if the user isn't
>> using Gnome at all?
>>
>
> From my research, Gnome seems to be the only window manager where this is
> an issue (it seems to be quite a controversial change). We will be working
> on a better, more permanent solution in the future, however that's a lot of
> effort, and not something we could hold up 3.0 for.
>
> Thanks.
>
> --
> Dave Page
> Blog: http://pgsnake.blogspot.com
> Twitter: @pgsnake
>
> EnterpriseDB UK: http://www.enterprisedb.com
> The Enterprise PostgreSQL Company
>

Re: Bug #3083 fix

[Joao De Almeida Pereira]

Hi Dave,
Do you still have issues in your environment?

Thanks
Joao

On Wed, Apr 11, 2018 at 5:56 AM Dave Page <dp...@pgadmin.org> wrote:

> Joao, are you looking at this?
>
> On Tue, Apr 3, 2018 at 2:03 PM, Harshal Dhumal <
> harshal.dhu...@enterprisedb.com> wrote:
>
>>
>>
>> --
>> *Harshal Dhumal*
>> *Sr. Software Engineer*
>>
>> EnterpriseDB India: http://www.enterprisedb.com
>> The Enterprise PostgreSQL Company
>>
>> On Tue, Apr 3, 2018 at 6:10 PM, Dave Page <dp...@pgadmin.org> wrote:
>>
>>> Argh, managed to send before I finished typing...
>>>
>>> On Tue, Apr 3, 2018 at 1:38 PM, Dave Page <dp...@pgadmin.org> wrote:
>>>
>>>> Hi
>>>>
>>>> On Thu, Mar 29, 2018 at 4:29 PM, Joao De Almeida Pereira <
>>>> jdealmeidapere...@pivotal.io> wrote:
>>>>
>>>>> Hi Dave,
>>>>> That looks like in the surrounding area of the change. We run our
>>>>> pipeline and everything was green.
>>>>> Can you provide more details, which python version are you using? OS?
>>>>>
>>>>
>>>> That was on my travel laptop, which is macOS Sierra with the Apple
>>>> supplied Python 2.7.
>>>>
>>>> Interestingly, I'm on my dev laptop today (same OS and Python) and it's
>>>> working just fine. The difference is that the travel machine is a 12"
>>>> Macbook, whilst the dev machine is
>>>>
>>>
>>> a 15" MacBook Pro with 2 24" external monitors. That makes me wonder if
>>> the small screen size is causing a problem with this test, something we
>>> have seen before.
>>>
>>>
>>
>> Yes, screen size does cause problem. Slick grid does not render all
>> columns if viewport is not wide enough (like it does for rows).
>> Remaining columns would render when user scrolls right.
>>
>> To avoid similar problem in datatype feature test (commit:
>> 88bcd3b5129db88975421e26c1bf188daf4892f9
>> <https://git.postgresql.org/gitweb/?p=pgadmin4.git;a=commitdiff;h=88bcd3b5129db88975421e26c1bf188daf4892f9>)
>> I have executed
>> queries in batch to limit number of columns in single query result.
>>
>>
>>
>>>
>>>>
>>>>>
>>>>> Thanks
>>>>> Joao
>>>>>
>>>>> On Thu, Mar 29, 2018 at 9:03 AM Dave Page <dp...@pgadmin.org> wrote:
>>>>>
>>>>>> Hi
>>>>>>
>>>>>> On Wed, Mar 28, 2018 at 7:06 PM, Joao De Almeida Pereira <
>>>>>> jdealmeidapere...@pivotal.io> wrote:
>>>>>>
>>>>>>> Hey Akshay and Neethu
>>>>>>>
>>>>>>> We refactored the patch to add tests for the resize feature.  We
>>>>>>> were able to write test cases for the drag event by using spies and 
>>>>>>> setting
>>>>>>> the rect dimensions.  In cases like this, we can just test some 
>>>>>>> components
>>>>>>> in order to have enough confidence in the code.  So we isolated the
>>>>>>> function that implements the behavior of this feature and tested that it
>>>>>>> was performing as expected.
>>>>>>>
>>>>>>> We ran the patch through the pipelines and all of the tests passed.
>>>>>>>
>>>>>>
>>>>>> I'm consistently seeing the feature test failure below with this
>>>>>> patch applied:
>>>>>>
>>>>>> ==
>>>>>> FAIL: runTest
>>>>>> (pgadmin.feature_tests.view_data_dml_queries.CheckForViewDataTest)
>>>>>> Validate Insert, Update operations in View/Edit data with given test
>>>>>> data
>>>>>> --
>>>>>> Traceback (most recent call last):
>>>>>>   File
>>>>>> "/Users/dpage/git/pgadmin4/web/pgadmin/feature_tests/view_data_dml_queries.py",
>>>>>> line 125, in runTest
>>>>>> self._verify_row_data(True)
>>>>>>   File
>>>>>> "/Users/dpage/git/pgadmin4/web/pgadmin/feature_tests/view_data_dml_queries.py",
>>>>>> line 325, in _verify_row_data
>>>>>> self.assertEquals(cells[idx], config_data[str(idx)][1])
>>>>>> AssertionError: u'[null]' != u'1'
>>>>>> - [null]
>>>>>> + 1
>>>>>>
>>>>>>
>>>>>> --
>>>>>> 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
>

Re: [pgadmin4][patch] Initial patch to decouple from ACI Tree

[Joao De Almeida Pereira]

Hello Khushboo

On Mon, Apr 9, 2018 at 1:59 AM Khushboo Vashi <
khushboo.va...@enterprisedb.com> wrote:

> Hi Joao,
>
> I have reviewed your patch and have some suggestions.
>
> On Sat, Apr 7, 2018 at 12:43 AM, Joao De Almeida Pereira <
> jdealmeidapere...@pivotal.io> wrote:
>
>> Hello Murtuza/Dave,
>> Yes now the extracted functions are spread into different files. The
>> intent would be to make the files as small as possible, and also to group
>> and name them in a way that would be easy to understand what each file is
>> doing without the need of opening it.
>> As a example:
>> static/js/backup will contain all the backup related functionality
>> inside of this folder we can see the file:
>>
> menu_utils.js At this moment in time we decided to group all the
>> functions that are related to the menu, but we can split that also if we
>> believe it is easier to see.
>>
> It's really very good to see the separated code for backup module. As we
> have done for backup, we would like do it for other PG utilities like
> restore, maintenance etc.
> Considering this, we should separate the code in a way that some of the
> common functionalities can be used for other modules  like menu (as you
> have mentioned above), dialogue factory etc.
> Also, I think these functionalities should be in their respective static
> folder instead of pgadmin/static.
>

About the location of the files. The move of the files to pgadmin/static/js
was made on purpose in order to clearly separate Javascript from python
code.
The rational behind it was
- Create a clear separation between the backend and frontend
- Having Javascript code concentrated in a single place, hopefully, will
encourage to developers to look for a functionality, that is already
implemented in another modules, because they are right there. (When we
started this journey we realized that the 'nodes' have a big groups of code
that could be shared, but because the Javascript is spread everywhere it is
much harder to look for it)


There are some drawbacks of this separation:
- When creating a new module we will need to put the javascript in a
separate location from the backend code


>
>
>> static/js/datagrid folder contains all the datagrid related functionality
>>
> Same as backup module,  this should be in it's respective static/js folder.
>
>> Inside of the folder we can see the files:
>> get_panel_title.js is responsible for retrieving the name of the panel
>> show_data.js is responsible for showing the datagrid
>> show_query_tool.js is responsible for showing the query tool
>>
>> Does this structure make sense?
>> Can you give an example of a comment that you think is missing and that
>> could help?
>>
>> As a personal note, unless the algorithm is very obscure or very
>> complicated, I believe that if the code needs comments it is a signal that
>> something needs to change in terms of naming, structure of the part in
>> question. This being said, I am open to add some comments that might help
>> people.
>>
> You are right, with the help of naming convention and structure of the
> code, any one can get the idea about the code. But it is very useful to
> understand the code
> very easily with the proper comments especially when there are multiple
> developers working on a single project.
>
> I found some of the places where it would be great to have comments.
>
> - treeMenu: new tree.Tree()  in a browser.js
> - tree.js  (especially Tree class)
>
About the comment point I need a more clear understanding on what kind of
comments you are looking for. Because when you read the function names you
understand the intent, what they are doing. The parameters also explain
what you need to pass into them.

If what you are looking for in these comments is the reasoning being the
change itself, then that should be present in the commit message. Specially
because this is going to be a very big patch with a very big number of
changes.

>
> Thanks
>> Joao
>> ​
>>
>> Thanks,
> Khushboo
>
>>
>> On Fri, Apr 6, 2018 at 4:48 AM Murtuza Zabuawala <
>> murtuza.zabuaw...@enterprisedb.com> wrote:
>>
>>> Hi Joao,
>>>
>>> Patch looks good and working as expected.
>>>
>>> I also agree with Dave, Can we please add some comments in each file
>>> which can help us to understand the flow, I'm saying because now the code
>>> is segregated in so many separate files it will be hard to keep track of
>>> the flow from one file to another when debugging.
>>>
>>>
>>> --
>>> Regards,
>>> Murtuza Zabuawala
>>> EnterpriseDB: http

Re: [pgAdmin4][RM#3257] Fix for explain functionality

[Joao De Almeida Pereira]

Hi Murtuza,
Looks like the tests are failing due to this change.
I also got some strange result on the linter, but I believe it is something
related to this machine.

Thanks
Joao

On Tue, Apr 10, 2018 at 6:20 AM Murtuza Zabuawala <
murtuza.zabuaw...@enterprisedb.com> wrote:

> Hi Dave,
>
> A minor fix for explain functionality.
>
> *Issue:* Due to multiline explain options in the SQL template CodeMirror
> was failing to highlight the exact line of error.
>
>
> --
> Regards,
> Murtuza Zabuawala
> EnterpriseDB: http://www.enterprisedb.com
> The Enterprise PostgreSQL Company
>
>

Re: [pgadmin4][patch] Initial patch to decouple from ACI Tree

[Joao De Almeida Pereira]

Hello Murtuza/Dave,
Yes now the extracted functions are spread into different files. The intent
would be to make the files as small as possible, and also to group and name
them in a way that would be easy to understand what each file is doing
without the need of opening it.
As a example:
static/js/backup will contain all the backup related functionality
inside of this folder we can see the file:
menu_utils.js At this moment in time we decided to group all the functions
that are related to the menu, but we can split that also if we believe it
is easier to see.

static/js/datagrid folder contains all the datagrid related functionality
Inside of the folder we can see the files:
get_panel_title.js is responsible for retrieving the name of the panel
show_data.js is responsible for showing the datagrid
show_query_tool.js is responsible for showing the query tool

Does this structure make sense?
Can you give an example of a comment that you think is missing and that
could help?

As a personal note, unless the algorithm is very obscure or very
complicated, I believe that if the code needs comments it is a signal that
something needs to change in terms of naming, structure of the part in
question. This being said, I am open to add some comments that might help
people.

Thanks
Joao
​


On Fri, Apr 6, 2018 at 4:48 AM Murtuza Zabuawala <
murtuza.zabuaw...@enterprisedb.com> wrote:

> Hi Joao,
>
> Patch looks good and working as expected.
>
> I also agree with Dave, Can we please add some comments in each file which
> can help us to understand the flow, I'm saying because now the code is
> segregated in so many separate files it will be hard to keep track of the
> flow from one file to another when debugging.
>
>
> --
> Regards,
> Murtuza Zabuawala
> EnterpriseDB: http://www.enterprisedb.com
> The Enterprise PostgreSQL Company
>
>
> On Thu, Apr 5, 2018 at 7:08 PM, Joao De Almeida Pereira <
> jdealmeidapere...@pivotal.io> wrote:
>
>> Hi Khushboo,
>> Attached you can find both patches rebased
>>
>> Thanks
>>
>>
>> On Thu, Apr 5, 2018 at 6:31 AM Khushboo Vashi <
>> khushboo.va...@enterprisedb.com> wrote:
>>
>>> Hi Joao,
>>>
>>> Can you please rebase the second patch?
>>>
>>> Thanks,
>>> Khushboo
>>>
>>>
>>> On Tue, Apr 3, 2018 at 12:15 AM, Joao De Almeida Pereira <
>>> jdealmeidapere...@pivotal.io> wrote:
>>>
>>>> Hi Hackers,
>>>>
>>>> Attached you can find the patch that will start to decouple pgAdmin
>>>> from ACITree library.
>>>> This patch is intended to be merged after 3.0, because we do not want
>>>> to cause any entropy or delay the release, but we want to start the
>>>> discussion and show some code.
>>>>
>>>> This job that we started is a massive tech debt chore that will take
>>>> some time to finalize and we would love the help of the community to do it.
>>>>
>>>> *Summary of the patch:*
>>>> 0001 patch:
>>>>  - Creates a new tree that will allow us to create a separation between
>>>> the application and ACI Tree
>>>>  - Creates a Fake Tree (Test double, for reference on the available
>>>> test doubles: https://martinfowler.com/bliki/TestDouble.html) that can
>>>> be used to inplace to replace the ACITree and also encapsulate the new tree
>>>> behavior on our tests
>>>>  - Adds tests for all the tree functionalities
>>>>
>>>> 0002 patch:
>>>>  - Extracts, refactors, adds tests and remove dependency from ACI Tree
>>>> on:
>>>> - getTreeNodeHierarchy
>>>> - on backup.js: menu_enabled, menu_enabled_server,
>>>> start_backup_global_server, backup_objects
>>>> - on datagrid.js: show_data_grid, get_panel_title, show_query_tool
>>>> - Start using sprintf-js as Underscore.String is deprecating sprintf
>>>> function
>>>>
>>>> This patch represents only 10 calls to ACITree.itemData out of 176 that
>>>> are spread around our code
>>>>
>>>>
>>>> *In Depth look on the process behind the patch:*
>>>>
>>>> We started writing this patch with the idea that we need to decouple
>>>> pgAdmin4 from ACITree, because ACITree is no longer supported, the
>>>> documentation is non existent and ACITree is no longer being actively
>>>> developed.
>>>>
>>>> Our process:
>>>> 1. We "randomly" selected a function that is part of the ACITree. From
>>>> this point we

Re: [pgAdmin4][RM#3072] Make pgagent job history rows configurable

[Joao De Almeida Pereira]

Hi Murtuza,
Generally the patch looks good passes all CI but the linter fails:

/tmp/build/4a5630c2/pivotal-rm-3072/web /tmp/build/4a5630c2
 

./pgadmin/browser/register_browser_preferences.py:11: [E302] expected
2 blank lines, found 1
 

1   E302 expected 2 blank lines, found 1
 

1


Did not test the functionality itself 

Thanks
Joao

On Thu, Apr 5, 2018 at 8:09 AM Murtuza Zabuawala <
murtuza.zabuaw...@enterprisedb.com> wrote:

> Hi Dave,
>
> Please find update patch.
>
> --
> Regards,
> Murtuza Zabuawala
> EnterpriseDB: http://www.enterprisedb.com
> The Enterprise PostgreSQL Company
>
>
> On Thu, Apr 5, 2018 at 4:52 PM, Murtuza Zabuawala <
> murtuza.zabuaw...@enterprisedb.com> wrote:
>
>> On Thu, Apr 5, 2018 at 4:43 PM, Dave Page  wrote:
>>
>>> Hi
>>>
>>> On Thu, Apr 5, 2018 at 11:10 AM, Murtuza Zabuawala <
>>> murtuza.zabuaw...@enterprisedb.com> wrote:
>>>
 Hi,

 PFA patch which allow user to configure how many rows they wish to
 display for any pgagent jobs on statistics panel.

>>>
>>> I think this is essentially good, however, I'm really not happy with the
>>> preference name and category. In general, I'd suggest that before creating
>>> patches in the future we should confirm naming etc on the mailing list, as
>>> I often end up changing wording and then requiring new screenshots etc.
>>>
>> ​Ok​
>>
>>
>>>
>>> In this case, I really don't like that we've added another category, and
>>> quite a specific one at that. I would suggest we move it to Browser ->
>>> Properties and name it "Maximum job history rows" with a description of
>>> "The maximum number of history rows to show on the Statistics tab for
>>> pgAgent jobs."
>>>
>> I'll change it and resend the patch.
>>
>>
>>>
>>> Thoughts?
>>>
>>> --
>>> Dave Page
>>> Blog: http://pgsnake.blogspot.com
>>> Twitter: @pgsnake
>>>
>>> EnterpriseDB UK: http://www.enterprisedb.com
>>> The Enterprise PostgreSQL Company
>>>
>>
>>
>

Re: pgAdmin4 - Issue of unmaintained libraries

[Joao De Almeida Pereira]

Hi Dave,

On Tue, Apr 3, 2018 at 8:32 AM Dave Page  wrote:

> Hi
>
> On Tue, Apr 3, 2018 at 1:22 PM, Murtuza Zabuawala <
> murtuza.zabuaw...@enterprisedb.com> wrote:
>
>> Hi Team,
>>
>> As we are heavily dependent on 3rd party JS libraries and some of them
>> are no longer actively maintained by their respective authors (Last commit
>> was approximate year ago).
>>
>> Some of the core libraries which are no longer actively maintained are,
>> - Backbone 
>> - wcDocker 
>> - aciTree 
>> - Backgrid /Backform
>> - jQuery 1.x 
>> version
>>
>> What would be our future plans when it comes to fixing the issues in the
>> core libraries or adding new feature?
>> (Ref: Email thread
>> 
>> )
>>
>
> Well jQuery can be updated can't it?
>
> wcDocker is, as far as I'm aware, the only library of it's kind. Unless
> something else has surpassed it in the last couple of years, there's
> nothing even close in functionality to it. aciTree was in a similar
> position, though Joao and team think there may be other candidates now.
>

About the wcDocker, in fact it as a lot of features. In our User Interviews
we didn't find any person that was using features like dragging and
creating new panels. The only feature that we see people using was the
tabs. So if the only feature that people use of wcDocker is really tabs
there are some other libraries like:
react-tabs or react-tabtab (Draggable tabs) or rc-tabs(Static tabs, with
52k Downloads last 7 days).

For ACITree we already started the process of isolating it from the
application, This will allow us in the future to replace it with something
that is more up to date.


> As for Backbone/Backgrid/Backform, I don't know. Backbone could maybe be
> replaced with React eventually. Not sure about the others.
>
> In any case, this is likely to be a problem on an ongoing basis - and I
> think we need to consider the future on a case by case basis when
> necessary. It may mean moving to a different library, or it may mean
> forking components, or it may be the upstream may have not had any commits
> for a long time simply because there is no development happening right now,
> but bugs may still be fixed.
>
>
We understand and agree that this should be handled case-by-case instead of
replacing everything at once since it is not feasible to do a complete
rewrite of the application.  But we also think it's hard to build new
features on top of libraries that are no longer supported.  We don't really
want to fork a library either because we'll become responsible for
maintaining that library, which means maintaining more code.  We want to
use libraries that are actively maintained so we can get security patches,
new features, etc.  Also, if we need a new functionality on a specific
library, we can, for example, open an issue and the maintainers of that
library can implement it for us or we can create a pull request for them to
merge.

Thanks,

Victoria & Joao


> --
> Dave Page
> Blog: http://pgsnake.blogspot.com
> Twitter: @pgsnake
>
> EnterpriseDB UK: http://www.enterprisedb.com
> The Enterprise PostgreSQL Company
>

Re: [pgAdmin4][RM#3154] Update modules to latest version

[Joao De Almeida Pereira]

Hi Murtuza,

The patches look good and they pass all tests in CI.
One think that we realized was the SlickGrid as a npm package now:
https://www.npmjs.com/package/slickgrid
Also Slickgrid comes packaged with jquery 3.1 not sure if it is fully
supported or not..

Thanks
Victoria & Joao

On Tue, Apr 3, 2018 at 11:50 AM Murtuza Zabuawala <
murtuza.zabuaw...@enterprisedb.com> wrote:

> ​Hi Dave,
>
> Reverted back jQuery version to 1.x because of SlickGrid dependancy.
> Please find updated patch.
>
> --
> Regards,
> Murtuza Zabuawala
> EnterpriseDB: http://www.enterprisedb.com
> The Enterprise PostgreSQL Company
>
>
> On Tue, Apr 3, 2018 at 8:50 PM, Murtuza Zabuawala <
> murtuza.zabuaw...@enterprisedb.com> wrote:
>
>> Please hold on my previous patch.
>>
>> We can't use latest jQuery version as SlickGrid has dependancy on older
>> version.
>> I'll send updated patch again.
>>
>> On Tue, Apr 3, 2018 at 8:20 PM, Murtuza Zabuawala <
>> murtuza.zabuaw...@enterprisedb.com> wrote:
>>
>>> Hi Dave,
>>>
>>> Please find updated patches, there are two patches attached one is for
>>> the story and another is for changes required as per new modules.
>>>
>>>
>>> On Tue, Apr 3, 2018 at 6:27 PM, Dave Page  wrote:
>>>
 Hi

 On Fri, Mar 30, 2018 at 8:01 AM, Murtuza Zabuawala <
 murtuza.zabuaw...@enterprisedb.com> wrote:

> Hi,
>
> PFA patch to update the modules to latest version.
>
>
 - Why isn't jQuery updated to 3.3.1?

>>> ​Done​
>>>
>>>
 ​


>>>
 - Shouldn't pkg/pip/setup_pip.py be updated with changes to psycopg2
 and pycrypto etc?

>>> ​Done​
>>>
>>>



> We are not able to update some of the modules to latest version due to
> dependancy on other modules, For example
> - Python: Flask-Security has dependancy on flask-babelex which causes
> conflict with flask_babel
>

 Hmm, flask-babelex might be a better option anyway; in particular, it
 avoids loading catalogs with every request which seems desirable given the
 size of ours. On the other hand, it hasn't been updated so recently.

>>> ​I have removed flask-babel and used flask_babelex instead.​
>>>
>>>


> - JS: Can't update to Bootstrap4 because Bootstrap Switch & Bootstrap 
> Datetime
> picker has dependancy on Bootstrap3.
>

 OK.


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

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

>>>
>>>
>>
>

Re: [pgAdmin4][RM#3235] Code refactoring in Query tool

[Joao De Almeida Pereira]

Hi Murtuza

It is really good to see other patches that are just refactoring code.

We have some suggestions:
- We are trying to use === instead of == because === ensure that the type
is also checked (
https://developer.mozilla.org/en-US/docs/Web/JavaScript/Equality_comparisons_and_sameness
)
- Now that we are refactoring some code, maybe we should keep some
consistency on the way we name functions and variables.
We should use camelCase for names instead of snake_case. In general, if you
see a function or variable name that doesn't fit the desired syntax or if a
block of code seems confusing, it is better to refactor it.
- By the name of the function is_new_transaction_required, it describes
what the function represents but doesn't seem to explain the full scope of
the function. What do you think about the name:
httpResponseRequiresNewTransaction
- The extraction doesn't look like it matches the code removed

-if (pgAdmin.Browser.UserManagement.is_pga_login_required(e)) {
-  self.save_state('_explain_timing', []);
-  return pgAdmin.Browser.UserManagement.pga_login();
-}
-
-if(transaction.is_new_transaction_required(e)) {
-  self.save_state('_explain_timing', []);
-  return self.init_transaction();
-}
-
-alertify.alert(gettext('Explain options error'),
-  gettext('Error occurred while setting timing option in
explain.')
+let msg = httpErrorHandler.handleQueryToolAjaxError(
+  pgAdmin, self, e, '_explain_timing', null, true
 );
+alertify.alert(gettext('Explain options error'), msg);
In this case we are only saving state if the following conditions are true:
isNotConnectedToThePythonServer and httpResponseJSONIsPresent and
connectionLostToPostgresDatabase and shouldSaveState
That is not the case on the removed code.
- The functions extracted when are called do not use all the parameters.
This tells us that the function groups too much functionality that is not
used in same cases. Maybe we should split this function into smaller
functions that do each part.


Thanks
Victoria & Joao

On Tue, Apr 3, 2018 at 11:38 AM Murtuza Zabuawala <
murtuza.zabuaw...@enterprisedb.com> wrote:

> Hi Dave,
>
> PFA updated patch, I've renamed it to query_tool_http_error_handler.js
> & query_tool_http_error_handler_spec.js respectively.
>
> --
> Regards,
> Murtuza Zabuawala
> EnterpriseDB: http://www.enterprisedb.com
> The Enterprise PostgreSQL Company
>
>
> On Tue, Apr 3, 2018 at 7:43 PM, Dave Page  wrote:
>
>> HI
>>
>> On Tue, Apr 3, 2018 at 12:27 PM, Murtuza Zabuawala <
>> murtuza.zabuaw...@enterprisedb.com> wrote:
>>
>>> Hi,
>>>
>>> PFA patch to extract the common code from query tool to handle ajax
>>> errors & connection handling, Also added unit tests around extracted code.
>>>
>>
>> Looks good to me, except, I wonder if we should rename
>> is_new_transaction_required.js/is_new_transaction_required_spec.js to
>> something a little more generic; maybe conn_tx_handler_funcs.js? Not sure I
>> like that though.
>>
>>
>> --
>> Dave Page
>> Blog: http://pgsnake.blogspot.com
>> Twitter: @pgsnake
>>
>> EnterpriseDB UK: http://www.enterprisedb.com
>> The Enterprise PostgreSQL Company
>>
>
>

Re: [pgAdmin4][RM#3155] Allow user to lock the Layout

[Joao De Almeida Pereira]

Hi Murtuza,

Everything seems to work, the tests are all green and the linter is fixed.

As we stated in an previous email, the direction we would love the
application to go is a more robust Javascript Front-End that would rely in
the Backend to provide data. Adding more things to templated Javascript
files feels like a step back and something that we will in the future have
to convert into AJAX calls and JSON responses.
As discussed before our idea is to remove all the javascript templated
files.

How hard do you think it would be to do this implementation without using
templates?

Thanks
Victoria & Joao

On Tue, Apr 3, 2018 at 7:57 AM Murtuza Zabuawala <
murtuza.zabuaw...@enterprisedb.com> wrote:

> Hi,
>
> Thanks Joao for reviewing.
>
> PFA updated patch.
>
> On Tue, Apr 3, 2018 at 1:11 AM, Joao De Almeida Pereira <
> jdealmeidapere...@pivotal.io> wrote:
>
>> Hello,
>>
>> On Mon, Apr 2, 2018 at 10:07 AM Murtuza Zabuawala <
>> murtuza.zabuaw...@enterprisedb.com> wrote:
>>
>>>
>>> ​Hello,
>>>
>>> Please find updated patch,
>>>
>>> Now layout will be locked after user updates its preferences, w
>>> e have used ​
>>> templated variable in the javascript file
>>> ​ because we do not have preference module or preference cache available
>>> when the page loads and panels gets rendered,
>>> ​I
>>> ​ also
>>> made changes in JS tests as per Joao's review comments.
>>>
>> Looks like everything is working when we change the lock.
>> As a personal preferences I would prefer to see this in at least 2
>> commits, one that is related to the preference issue and another one that
>> is related to this story.
>>
>>
>> All the tests are working, but he linter is failing:
>>
>> /tmp/build/4a5630c2/pivotal-rm-3155/web /tmp/build/4a5630c2
>>  
>> <https://gpdb-dev.bosh.pivotalci.info/teams/main/pipelines/pgadmin-feature-branches/jobs/pivotal-rm-3155-python-linter/builds/3#L5ab982d1:9>
>> ./pgadmin/misc/__init__.py:78: [E303] too many blank lines (2)
>>  
>> <https://gpdb-dev.bosh.pivotalci.info/teams/main/pipelines/pgadmin-feature-branches/jobs/pivotal-rm-3155-python-linter/builds/3#L5ab982d1:10>
>> 1   E303 too many blank lines (2)
>>  
>> <https://gpdb-dev.bosh.pivotalci.info/teams/main/pipelines/pgadmin-feature-branches/jobs/pivotal-rm-3155-python-linter/builds/3#L5ab982d1:11>
>>
>> 1
>>
> ​Fixed​
>
>
>>
>>
>>> @Dave/Pivotal team,
>>> The given patch is working fine for all the Tabs/Panels (all the panels
>>> from main window as well as from Query tool and Debugger) but I'm facing an
>>> issue while handling the Browser tree section, It is a wcDocer frame
>>> <http://docker.api.webcabin.org/module-wcFrame.html> and not a wcDocker
>>> panel <http://docker.api.webcabin.org/module-wcPanel.html>. Like
>>> wcDocker panel, wcDocker frame do not provide any API so that a developer
>>> can prevent drag-drop functionality on it.
>>>
>>> By visiting wcDocker github page <https://github.com/WebCabin/wcDocker> It
>>> looks like it not actively maintained.
>>> What do you suggest how should we tackle this issue?
>>>
>>>
>> I think this should be moved to a different thread, because at this point
>> in time we have 3 of our core libraries that are no longer
>> maintained/supported/under active development that I know out of my head.
>> (ACITree, Backbone and wcDocker). I might even add to the mix jquery 1.11.2
>> because it stopped being actively developed and supported after May 20 of
>> 2016.
>>
> ​Sure, I'll send separate email.​
>
>
>>
>>
>>> For time being, I've created subtask for this issue
>>> https://redmine.postgresql.org/issues/3243
>>>
>>> Thanks,
>>> Murtuza
>>>  ​
>>> On Thu, Mar 29, 2018 at 8:57 PM, Joao De Almeida Pereira <
>>> jdealmeidapere...@pivotal.io> wrote:
>>>
>>>> Hi Murtuza,
>>>>
>>>> After changing the setting in the preferences nothing happened, we had
>>>> to reset the layout or refresh the app to see it working. It only looks the
>>>> right side. Was this the intended behavior?
>>>>
>>>> Not sure if this is the expected behavior or not. I would expect that
>>>> any change I do in the preferences would start working after I press the
>>>> Save button. This also happens with other preferences that only take effect
>>>> after refresh on the

[pgadmin4][patch] #3244 Query elapse time granularity

[Joao De Almeida Pereira]

Hi Hackers,

Attached you can find a patch that increases the granularity of time
displayed for total run time of a query.
Also extracts the functionality, wraps it with tests.
An addon to this patch is the extraction of the function
call_render_after_poll  that uses the function that calculates the time as
well


Thanks
Joao


granularity-of-query-elapse-time.diff
Description: Binary data

Re: [pgAdmin4][Patch]: RM #3180 Index node is missing from the tree view of the table node

[Joao De Almeida Pereira]

Hi Murtuza,
Lets imagine you have

CREATE TABLE cities (
city_id bigserial not null,
name text not null,
population   int
) PARTITION BY LIST (initcap(name));

CREATE TABLE cities_west
PARTITION OF cities (
CONSTRAINT city_id_nonzero CHECK (city_id != 0)
) FOR VALUES IN ('Los Angeles', 'San Francisco') PARTITION BY RANGE
(population);

CREATE TABLE cities_west_1_to_10
PARTITION OF cities_west FOR VALUES FROM (1) TO (10);

​

You can only create an index in *cities_west_1_to_10* because
postgresql assumes that *cities_west* is also a partitioned table. So the
implementation looks correct, despite the fact that there are no tests
around it.

Thanks
Joao

On Thu, Mar 29, 2018 at 9:26 AM Akshay Joshi 
wrote:

> Hi Murtuza
>
> On Thu, Mar 29, 2018 at 6:36 PM, Murtuza Zabuawala <
> murtuza.zabuaw...@enterprisedb.com> wrote:
>
>> Hi Akshay,
>>
>> I have concerns regarding the fix, As you negate the condition, Before
>> the fix it was not displaying Index node for Tables but after the fix it
>> will not display it for Partition tables.
>> But when I read the Postgres docs it say,
>> *Partitions may themselves be defined as partitioned tables, using what
>> is called sub-partitioning. Partitions may have their own indexes,
>> constraints and default values, distinct from those of other partitions.
>> Indexes must be created separately for each partition. See CREATE TABLE
>>  for more
>> details on creating partitioned tables and partitions.*
>> Ref: https://www.postgresql.org/docs/10/static/ddl-partitioning.html
>> (Sec: 5.10.2)
>>
>
> Yes that is correct, but it's about Partitions(child tables), not the
> *Partitioned* table. We are showing indexes on Partitions. Please refer
> "Index_on_Partitioned_table.png"
>
>>
>>
>>
>> --
>> Regards,
>> Murtuza Zabuawala
>> EnterpriseDB: http://www.enterprisedb.com
>> The Enterprise PostgreSQL Company
>>
>>
>> On Thu, Mar 29, 2018 at 6:05 PM, Akshay Joshi <
>> akshay.jo...@enterprisedb.com> wrote:
>>
>>> Hi Hackers,
>>>
>>> Please find the attached patch to fix RM #3180 Index node is missing
>>> from the tree view of the table node. This is a regression of one of the
>>> older commit.
>>>
>>> --
>>> *Akshay Joshi*
>>>
>>> *Sr. Software Architect *
>>>
>>>
>>>
>>> *Phone: +91 20-3058-9517 <+91%2020%203058%209517>Mobile: +91
>>> 976-788-8246 <+91%2097678%2088246>*
>>>
>>
>>
>
>
> --
> *Akshay Joshi*
>
> *Sr. Software Architect *
>
>
>
> *Phone: +91 20-3058-9517 <+91%2020%203058%209517>Mobile: +91 976-788-8246
> <+91%2097678%2088246>*
>

Re: Bug #3083 fix

[Joao De Almeida Pereira]

Hi Dave,
That looks like in the surrounding area of the change. We run our pipeline
and everything was green.
Can you provide more details, which python version are you using? OS?

Thanks
Joao

On Thu, Mar 29, 2018 at 9:03 AM Dave Page <dp...@pgadmin.org> wrote:

> Hi
>
> On Wed, Mar 28, 2018 at 7:06 PM, Joao De Almeida Pereira <
> jdealmeidapere...@pivotal.io> wrote:
>
>> Hey Akshay and Neethu
>>
>> We refactored the patch to add tests for the resize feature.  We were
>> able to write test cases for the drag event by using spies and setting the
>> rect dimensions.  In cases like this, we can just test some components in
>> order to have enough confidence in the code.  So we isolated the function
>> that implements the behavior of this feature and tested that it was
>> performing as expected.
>>
>> We ran the patch through the pipelines and all of the tests passed.
>>
>
> I'm consistently seeing the feature test failure below with this patch
> applied:
>
> ==
> FAIL: runTest
> (pgadmin.feature_tests.view_data_dml_queries.CheckForViewDataTest)
> Validate Insert, Update operations in View/Edit data with given test data
> --
> Traceback (most recent call last):
>   File
> "/Users/dpage/git/pgadmin4/web/pgadmin/feature_tests/view_data_dml_queries.py",
> line 125, in runTest
> self._verify_row_data(True)
>   File
> "/Users/dpage/git/pgadmin4/web/pgadmin/feature_tests/view_data_dml_queries.py",
> line 325, in _verify_row_data
> self.assertEquals(cells[idx], config_data[str(idx)][1])
> AssertionError: u'[null]' != u'1'
> - [null]
> + 1
>
>
> --
> Dave Page
> Blog: http://pgsnake.blogspot.com
> Twitter: @pgsnake
>
> EnterpriseDB UK: http://www.enterprisedb.com
> The Enterprise PostgreSQL Company
>

Re: [pgAdmin4][RM#3155] Allow user to lock the Layout

[Joao De Almeida Pereira]

Hi Murtuza,

After changing the setting in the preferences nothing happened, we had to
reset the layout or refresh the app to see it working. It only looks the
right side. Was this the intended behavior?

Not sure if this is the expected behavior or not. I would expect that any
change I do in the preferences would start working after I press the Save
button. This also happens with other preferences that only take effect
after refresh on the browser.
This being said, not sure if having the templated variable in the
javascript file is the best approach in this case.

Do you think you can remove the requirejs tags on the tests?

At the testing file you do not need to create 3 different variables for the
panels, you can reuse it, because the beforeEach will run for every test

Thanks
Joao

On Thu, Mar 29, 2018 at 9:48 AM Dave Page  wrote:

> Hi
>
> On Thu, Mar 29, 2018 at 2:15 PM, Murtuza Zabuawala <
> murtuza.zabuaw...@enterprisedb.com> wrote:
>
>> Hi,
>>
>> PFA patch which will allow user to lock the panels and it will not allow
>> user to drag & drop them.
>>
>
> Tests pass, but when I lock the layout, I can still drag panels and adjust
> the splitters etc. After doing so,  reset the layout and now have the
> broken layout seen in the attached screenshot. I have rebuilt the bundle,
> reloaded etc.
>
> --
> Dave Page
> Blog: http://pgsnake.blogspot.com
> Twitter: @pgsnake
>
> EnterpriseDB UK: http://www.enterprisedb.com
> The Enterprise PostgreSQL Company
>

Re: Bug #3083 fix

[Joao De Almeida Pereira]

Hey Akshay and Neethu

We refactored the patch to add tests for the resize feature.  We were able
to write test cases for the drag event by using spies and setting the rect
dimensions.  In cases like this, we can just test some components in order
to have enough confidence in the code.  So we isolated the function that
implements the behavior of this feature and tested that it was performing
as expected.

We ran the patch through the pipelines and all of the tests passed.

Sincerely,

Joao and Victoria

On Wed, Mar 28, 2018 at 8:03 AM Akshay Joshi <akshay.jo...@enterprisedb.com>
wrote:

> Hi
>
> On Fri, Mar 2, 2018 at 3:40 AM, Joao De Almeida Pereira <
> jdealmeidapere...@pivotal.io> wrote:
>
>> Hello Neethu,
>> We passed the patch through our CI pipeline and all tests pass.
>> The code looks good, but we are trying to decouple files as much  as we
>> can so that we do not end up with files with over 1000 lines, that are hard
>> to read and to maintain. Also we are trying to create Unit Tests to have
>> more coverage in our Javascript code.
>>
>> Can you split the new implementation code into it's own file and create
>> some tests to ensure the behavior will not be broken in the future?iYou
>> have some examples
>> on: pgadmin/browser/server_groups/servers/databases/external_tables/*
>>
>
> I have spilt the new implementation into different file. Its' been
> hard to write jasmine/feature test case as it requires drag event and exact
> co-ordinate to resize the slickgrid cell.
> Attached is the modified patch.
>
>
>>
>> Thanks
>> Joao
>>
>> On Thu, Mar 1, 2018 at 10:37 AM Neethu Mariya Joy <
>> neethumariya...@gmail.com> wrote:
>>
>>> Hi,
>>> I am Neethu Mariya Joy, an undergraduate pursuing BE in Computer Science
>>> at BITS Pilani.
>>>
>>> I've attempted to fix https://redmine.postgresql.org/issues/3083. Since
>>> the textarea resize feature is the default HTML feature, I have not changed
>>> it. Instead, I've added draggable borders to the wrapper which expands the
>>> textarea inside it.
>>>
>>> I'm attaching my patch as bug3083.diff below as per the contribution
>>> guidelines.
>>>
>>> Hope this helps. Thank you for your consideration!
>>>
>>> Sincerely,
>>> Neethu Mariya Joy
>>> GitHub <https://github.com/Roboneet> | Linkedin
>>> <https://www.linkedin.com/in/neethu-mariya-joy-653655128/>
>>>
>>>
>>>
>
>
> --
> *Akshay Joshi*
>
> *Sr. Software Architect *
>
>
>
> *Phone: +91 20-3058-9517 <+91%2020%203058%209517>Mobile: +91 976-788-8246
> <+91%2097678%2088246>*
>
diff --git a/web/pgadmin/static/js/slickgrid/editors.js b/web/pgadmin/static/js/slickgrid/editors.js
index 7652bf3b..a7df4baf 100644
--- a/web/pgadmin/static/js/slickgrid/editors.js
+++ b/web/pgadmin/static/js/slickgrid/editors.js
@@ -3,8 +3,9 @@
  * @module Editors
  * @namespace Slick
  */
+import {resizeContentOnDrag} from 'resize_editor';
 
-(function($) {
+(function ($) {
   // register namespace
   $.extend(true, window, {
 'Slick': {
@@ -52,6 +53,7 @@
 val = $.trim(val);
 return !(val != '' && (val.charAt(0) != '{' || val.charAt(val.length - 1) != '}'));
   }
+
   /*
* This function handles the [default] and [null] values for cells
* if row is copied, otherwise returns the editor value.
@@ -129,7 +131,7 @@
 var defaultValue;
 var scope = this;
 
-this.init = function() {
+this.init = function () {
   var $container = $('body');
 
   $wrapper = getWrapper().appendTo($container);
@@ -140,11 +142,12 @@
   $buttons.find('button:last').on('click', this.cancel);
   $input.bind('keydown', this.handleKeyDown);
 
+  resizeContentOnDrag($wrapper, $input);
   scope.position(args.position);
   $input.focus().select();
 };
 
-this.handleKeyDown = function(e) {
+this.handleKeyDown = function (e) {
   if (e.which == $.ui.keyCode.ENTER && e.ctrlKey) {
 scope.save();
   } else if (e.which == $.ui.keyCode.ESCAPE) {
@@ -159,40 +162,40 @@
   }
 };
 
-this.save = function() {
+this.save = function () {
   args.commitChanges();
 };
 
-this.cancel = function() {
+this.cancel = function () {
   $input.val(defaultValue);
   args.cancelChanges();
 };
 
-this.hide = function() {
+this.hide = function () {
   $wrapper.hide();
 };
 
-this.show = function() {
+this.show = function () {
   $wrapper.show();
 };
 
-this.position = function(position) {
+this.position = function (position) {
   calculateEditorPosition(position, $wrapper);
   $wrappe

Re: [pgAdmin4][patch]: RM #3090 pgadmin shows misleading "Query returned successfully" with incorrect SQL

[Joao De Almeida Pereira]

Hi Akshay,
Thanks for the clarification, looks like there isn't much we can do then

Best Regards,
Joao

On Wed, Mar 28, 2018 at 1:35 AM Akshay Joshi <akshay.jo...@enterprisedb.com>
wrote:

> Hi Joao
>
> On Tue, Mar 27, 2018 at 6:53 PM, Joao De Almeida Pereira <
> jdealmeidapere...@pivotal.io> wrote:
>
>> Hi Akshay,
>>
>> We were not trying to imply that your fix did not solve the problem, we
>> were trying to understand the root cause of the problem.
>>
>
> As per my understanding root cause of the problem is  "exception_obj.
> diag.severity" variable was not decoded and used directly, when I debug
> the code that variable contains the encoded string, so I have just use the
> function "*self.decode_to_utf8()*" and it works. If you can see the code
> below to this fix will have all the variables decoded to utf8.
>
>>
>> 1. We were not able to reproduce the problem
>> We followed your directions in RM, removed your fix but we could not
>> reproduce the problem. So we could not make sure that the application is in
>> fact working. This maybe have been because we missed something.
>> 2. The fix does not tackle the big problem
>> From what we read on the RM the big problem is "when we throw an
>> exception the front end is displaying the query successful message". Did
>> you also understood that from the RM?
>>
> We believe this is the real problem behind the RM.
>> Were you able replicate this behavior? If so we need to address the root
>> of this.
>>
>
> Yes I understood that from RM, but as I mentioned in my previous email
> that I haven't encounter that problem while trying to reproduce this. I got
> "*Not connected to the server*..." error not the "Query returned
> successfully". This bug has been log 2 months ago, I am not sure but it may
> possible that we have fixed/change some logic because of that I have seen
> "Not connected to the server.." error instead of "Query returned
> successfully".
>
>>
>>
>> As an aside sniping bugs is fine in some situations but as a general rule
>> of thumb is a bad approach and creates a blob of code that no one can
>> manage. A situation like this look like a very promising candidate for
>> extraction/refactoring in our point of view. In which situation do you
>> normally refactor or extract code?
>>
>> Thanks
>> Joao
>>
>>
>> On Tue, Mar 27, 2018, 1:31 AM Akshay Joshi <akshay.jo...@enterprisedb.com>
>> wrote:
>>
>>> Hi Joao
>>>
>>> On Tue, Mar 27, 2018 at 12:01 AM, Joao De Almeida Pereira <
>>> jdealmeidapere...@pivotal.io> wrote:
>>>
>>>> Hello,
>>>> We tried to reproduce the issue but we were not capable to reproduce
>>>> it.
>>>> What it is strange on the fix is that python is complaining about a
>>>> different line then the one that was fixed. Maybe this is just a Python
>>>> thing
>>>>
>>>
>>> I have mentioned the steps in RM and I have tried it on Ubuntu 16.
>>> Python is complaining the exact line where I have fixed the logic.
>>>
>>>ex_diag_message = u"{0}:  {1}".format(
>>>  self.decode_to_utf8(exception_obj.diag.severity), # 
>>> exception_obj.diag.severity is not decoded before my fix.
>>>  self.decode_to_utf8(exception_obj.diag.message_primary)
>>>)
>>>
>>>
>>>
>>>>
>>>> I assume that the fix works, but I would love to see some tests to
>>>> ensure it is working. Another issue that looks more problematic is the fact
>>>> that, as per the Redmine issue, when an exception is thrown it sends back a
>>>> Successful Query message. If this is the case then this fix doesn't look
>>>> like it is enough to solve the problem.
>>>>
>>>
>>> Yes it works. With the latest code I didn't see Successful Query
>>> message, it is showing "Not connected to the server .", but the stack
>>> trace is same that was mentioned in the RM.
>>>
>>>>
>>>> Thanks
>>>> Victoria & Joao
>>>>
>>>> On Mon, Mar 26, 2018 at 9:00 AM Dave Page <dp...@pgadmin.org> wrote:
>>>>
>>>>> Thanks, applied.
>>>>>
>>>>> On Mon, Mar 26, 2018 at 11:43 AM, Akshay Joshi <
>>>>> akshay.jo...@enterprisedb.com> wrote:
>>>>>
>>>>>> Hi Hackers,
>>>>>>
>>>>>> Please find the attached patch to fix RM #3090 pgadmin shows
>>>>>> misleading "Query returned successfully" with incorrect SQL.
>>>>>>
>>>>>> --
>>>>>> *Akshay Joshi*
>>>>>>
>>>>>> *Sr. Software Architect *
>>>>>>
>>>>>>
>>>>>>
>>>>>> *Phone: +91 20-3058-9517 <+91%2020%203058%209517>Mobile: +91
>>>>>> 976-788-8246 <+91%2097678%2088246>*
>>>>>>
>>>>>
>>>>>
>>>>>
>>>>> --
>>>>> Dave Page
>>>>> Blog: http://pgsnake.blogspot.com
>>>>> Twitter: @pgsnake
>>>>>
>>>>> EnterpriseDB UK: http://www.enterprisedb.com
>>>>> The Enterprise PostgreSQL Company
>>>>>
>>>>
>>>
>>>
>>> --
>>> *Akshay Joshi*
>>>
>>> *Sr. Software Architect *
>>>
>>>
>>>
>>> *Phone: +91 20-3058-9517 <+91%2020%203058%209517>Mobile: +91
>>> 976-788-8246 <+91%2097678%2088246>*
>>>
>>
>
>
> --
> *Akshay Joshi*
>
> *Sr. Software Architect *
>
>
>
> *Phone: +91 20-3058-9517 <+91%2020%203058%209517>Mobile: +91 976-788-8246
> <+91%2097678%2088246>*
>

Re: [pgAdmin4][patch]: RM #3090 pgadmin shows misleading "Query returned successfully" with incorrect SQL

[Joao De Almeida Pereira]

Hi Akshay,

We were not trying to imply that your fix did not solve the problem, we
were trying to understand the root cause of the problem.

1. We were not able to reproduce the problem
We followed your directions in RM, removed your fix but we could not
reproduce the problem. So we could not make sure that the application is in
fact working. This maybe have been because we missed something.
2. The fix does not tackle the big problem
>From what we read on the RM the big problem is "when we throw an exception
the front end is displaying the query successful message". Did you also
understood that from the RM?
We believe this is the real problem behind the RM.
Were you able replicate this behavior? If so we need to address the root of
this.


As an aside sniping bugs is fine in some situations but as a general rule
of thumb is a bad approach and creates a blob of code that no one can
manage. A situation like this look like a very promising candidate for
extraction/refactoring in our point of view. In which situation do you
normally refactor or extract code?

Thanks
Joao

On Tue, Mar 27, 2018, 1:31 AM Akshay Joshi <akshay.jo...@enterprisedb.com>
wrote:

> Hi Joao
>
> On Tue, Mar 27, 2018 at 12:01 AM, Joao De Almeida Pereira <
> jdealmeidapere...@pivotal.io> wrote:
>
>> Hello,
>> We tried to reproduce the issue but we were not capable to reproduce it.
>> What it is strange on the fix is that python is complaining about a
>> different line then the one that was fixed. Maybe this is just a Python
>> thing
>>
>
> I have mentioned the steps in RM and I have tried it on Ubuntu 16.
> Python is complaining the exact line where I have fixed the logic.
>
>ex_diag_message = u"{0}:  {1}".format(
>  self.decode_to_utf8(exception_obj.diag.severity), # 
> exception_obj.diag.severity is not decoded before my fix.
>  self.decode_to_utf8(exception_obj.diag.message_primary)
>)
>
>
>
>>
>> I assume that the fix works, but I would love to see some tests to ensure
>> it is working. Another issue that looks more problematic is the fact that,
>> as per the Redmine issue, when an exception is thrown it sends back a
>> Successful Query message. If this is the case then this fix doesn't look
>> like it is enough to solve the problem.
>>
>
> Yes it works. With the latest code I didn't see Successful Query
> message, it is showing "Not connected to the server .", but the stack
> trace is same that was mentioned in the RM.
>
>>
>> Thanks
>> Victoria & Joao
>>
>> On Mon, Mar 26, 2018 at 9:00 AM Dave Page <dp...@pgadmin.org> wrote:
>>
>>> Thanks, applied.
>>>
>>> On Mon, Mar 26, 2018 at 11:43 AM, Akshay Joshi <
>>> akshay.jo...@enterprisedb.com> wrote:
>>>
>>>> Hi Hackers,
>>>>
>>>> Please find the attached patch to fix RM #3090 pgadmin shows
>>>> misleading "Query returned successfully" with incorrect SQL.
>>>>
>>>> --
>>>> *Akshay Joshi*
>>>>
>>>> *Sr. Software Architect *
>>>>
>>>>
>>>>
>>>> *Phone: +91 20-3058-9517 <+91%2020%203058%209517>Mobile: +91
>>>> 976-788-8246 <+91%2097678%2088246>*
>>>>
>>>
>>>
>>>
>>> --
>>> Dave Page
>>> Blog: http://pgsnake.blogspot.com
>>> Twitter: @pgsnake
>>>
>>> EnterpriseDB UK: http://www.enterprisedb.com
>>> The Enterprise PostgreSQL Company
>>>
>>
>
>
> --
> *Akshay Joshi*
>
> *Sr. Software Architect *
>
>
>
> *Phone: +91 20-3058-9517 <+91%2020%203058%209517>Mobile: +91 976-788-8246
> <+91%2097678%2088246>*
>

Re: [pgAdmin4][Patch]: RM #1978 - Add an option to allow user to disable alertifyjs and acitree animations

[Joao De Almeida Pereira]

Hi Khushboo,

Looks like you have a typo on your CSS where it reads 'zoomeIn' it should
be 'zoomIn'.
Also setting more parameters into the window, in our experience, is never
good, so maybe it is time to create a real settings cache that can retrieve
from the backend the settings, like this one.

Thanks
Victoria & Joao

On Mon, Mar 26, 2018 at 8:38 AM Dave Page  wrote:

> Hi
>
> On Mon, Mar 26, 2018 at 7:23 AM, Khushboo Vashi <
> khushboo.va...@enterprisedb.com> wrote:
>
>> Hi,
>>
>> Please find the attached patch to fix RM #1978: Add an option to allow
>> user to disable alertifyjs and acitree animations.
>>
>
> I think these really need to be per-user settings, not per-installation..
> Whether or not animations are shown is really a matter of personal taste
> and circumstance.
>
> Thanks.
>
> --
> Dave Page
> Blog: http://pgsnake.blogspot.com
> Twitter: @pgsnake
>
> EnterpriseDB UK: http://www.enterprisedb.com
> The Enterprise PostgreSQL Company
>

Re: [pgAdmin4][patch]: RM #3090 pgadmin shows misleading "Query returned successfully" with incorrect SQL

[Joao De Almeida Pereira]

Hello,
We tried to reproduce the issue but we were not capable to reproduce it.
What it is strange on the fix is that python is complaining about a
different line then the one that was fixed. Maybe this is just a Python
thing

I assume that the fix works, but I would love to see some tests to ensure
it is working. Another issue that looks more problematic is the fact that,
as per the Redmine issue, when an exception is thrown it sends back a
Successful Query message. If this is the case then this fix doesn't look
like it is enough to solve the problem.

Thanks
Victoria & Joao

On Mon, Mar 26, 2018 at 9:00 AM Dave Page  wrote:

> Thanks, applied.
>
> On Mon, Mar 26, 2018 at 11:43 AM, Akshay Joshi <
> akshay.jo...@enterprisedb.com> wrote:
>
>> Hi Hackers,
>>
>> Please find the attached patch to fix RM #3090 pgadmin shows misleading
>> "Query returned successfully" with incorrect SQL.
>>
>> --
>> *Akshay Joshi*
>>
>> *Sr. Software Architect *
>>
>>
>>
>> *Phone: +91 20-3058-9517 <+91%2020%203058%209517>Mobile: +91 976-788-8246
>> <+91%2097678%2088246>*
>>
>
>
>
> --
> Dave Page
> Blog: http://pgsnake.blogspot.com
> Twitter: @pgsnake
>
> EnterpriseDB UK: http://www.enterprisedb.com
> The Enterprise PostgreSQL Company
>

Re: [pgAdmin4][RM#3055] Allow user to sort the data in View data mode

[Joao De Almeida Pereira]

Hi Hackers,

@Murtuza: The patch codewise looks good. Nice to see that we are using
axios instead of jquery ajax calls and that there is some coverage for the
change.
Nevertheless the Javascript testing looks a bit slim and could be improved.
Also the DataSorting class could have some other member functions like the
model validation could be extracted out so that it is easily tested.


@Hackers: This was how we tried to test this feature:
1 - Started pgAdmin
2 - Opened the query tool for a specific server
3 - Executed a SQL statment
4 - Pressed the column header to try to order, nothing happened
5 - Right clicked the column header to see if it was there the option,
nothing

This is the behavior that we were expecting, not to have to open Data View
and then press the icon that is not even near the grid in order to sort the
column. Is this really the way we want people to use the grid in pgAdmin?
Should it be more intuitive?


PS: Also that Orange after the selection is like a push in the eyes and not
in a good way. Maybe we should think about changing the color of the icon
to blue to match the rest of the website or something.

Thanks
Victoria & Joao

On Mon, Mar 26, 2018 at 12:13 PM Murtuza Zabuawala <
murtuza.zabuaw...@enterprisedb.com> wrote:

> On Mon, Mar 26, 2018 at 5:52 PM, Dave Page  wrote:
>
>> Hi
>>
>> On Sun, Mar 25, 2018 at 7:13 PM, Murtuza Zabuawala <
>> murtuza.zabuaw...@enterprisedb.com> wrote:
>>
>>> Hi,
>>>
>>> PFA patch which allow user to sort the data in View data mode.
>>>
>>
>> The patch looks good in general, however I'm not sure about the UI, in
>> particular that the closely-linked dialogue for filtering is a completely
>> different design. I think it would be better to combine the Sort/Filter
>> options and use a single dialogue for both, as pgAdmin 3 did (though, maybe
>> not using separate tabs for each part, but the top and bottom of the same
>> dialogue.
>>
>> That would certainly fix the consistency of the dialogues (obviously, as
>> there would only be one!), and I think would perhaps be a more simple
>> overall UI, particularly for those that want to sort and filter.
>>
> ​Sure, I'll send updated it accordingly.​
>
>
>>
>> Thoughts?
>>
>> Thanks.
>>
>> --
>> Dave Page
>> Blog: http://pgsnake.blogspot.com
>> Twitter: @pgsnake
>>
>> EnterpriseDB UK: http://www.enterprisedb.com
>> The Enterprise PostgreSQL Company
>>
>

Re: Showstopper desktop runtime issue

[Joao De Almeida Pereira]

Hello Hackers

In the PoC we did with Electron last month this was the point where we
stopped

What did we accomplish:
 - Use Electron as a runtime and packaging solution for the application
 - Support opening new windows when the preferences ask for them
 - Make the application generally work
 - Use Chrome as a default browser for the application
 - Building scripts for all platforms using `yarn`, please review Readme
 - The packaged Python version is 3.6 to Mac and Linux, and 3.4 to Windows
 - In our point of view it is a simpler way to generate the installers

Work in progress:
 - No random port for the server, so you can only start 1 instance per
machine
 - Tab support, there is no native support for tabs in Electron. It is
possible to do that, and eventually you will see a option in the menu to
create a new tab, but for this PoC we decided to disable the creation of
tabs. Tabs need to be implemented using HTML and cannot be ripped of from
the current window, like in Chrome
 - Did not test in CentOS, but tested in Ubuntu and it is working (We tried
but the electron required GLIBC 2.25 that was not on the version of CentOS
that we had)
 - In Linux despite the fact that we close the window, the application is
still running and need to be killed by hand
 - We didn't test Debugger opening in new window
 - Loading screen has no reference to pgAdmin
 - No logging file for the runtime
 - Windows we are using python 3.4 and using prior version of psycopg2
2.5.6 and pycrypto 2.6.1 (The version of psycopg2 is not the correct one,
this is because we couldn't find a precompile version of psycopg2)
 - This is just a spike and the code looks pretty messy. This need to be
changed in order to make it more readable and have some tests around it
 - Murtuza and Fahar were not able to run the application in Windows
 - Khushboo was not able to run Ubuntu version

Given this was the status at the time, we need to pick and choose what we
really want to implement right out of the box and what could be postponed.
In order to generate Windows build and ensure that the Linux build is
working we need so help. If we could have access to the machines that you
use to build pgAdmin or access to the venv folder that is generated in
these machines we could try to create a build from that.

The changes we produced still applies cleanly on master and can be found
in: https://github.com/greenplum-db/pgadmin4/tree/electron-over-master

Thanks
Victoria & Joao

On Thu, Mar 22, 2018 at 12:58 PM Dave Page  wrote:

> On Thu, Mar 22, 2018 at 4:28 PM, Dave Page  wrote:
>
>>
>> 2) This option is not very appealing to me, because we would be pilling
 code into the QT portion of the application, that I hope we remove in the
 future, that currently is untested to solve a problem caused by a Window
 Manager..
 I would be more in favor of creating a Application Indicator that would
 support 2 actions, Kill running pgAdmins and launch the browser to access
 them. There is a lot of interesting websites that talk about this, and how
 to develop. I had to download one to have a docker indicator. But as I
 said in a previous thread, I believe that this should be a 3rd party
 application and not a first class citizen on pgAdmin, as the majority of
 the ones that I found are.

>>>
>>> I can't comment on the specific ways to sort it out, but I think
>>> *basing* things in option 2 is by far the best option. If it's just an
>>> additional add-on that can be made a dependency of the packages it's not a
>>> huge problem (provided this add-on is available on the major platforms like
>>> rhel, ubuntu, debian of course)
>>>
>>
>> It would just be a modified version of what we have. Instead of having an
>> icon in the system tray, we'd probably have multiple Start Menu icons to
>> replace the tray icon menu. They would have to signal a running instance to
>> do something, or become a new instance and then do the something if nothing
>> is running already.
>>
>
> Of course, another option here might be to figure out that we're running
> under Gnome/GTK at runtime, and if so, create an indicator icon and menu
> instead of the tray icon. That is, apparently, what Skype and other similar
> apps do now. The indicator icons go on the right of the top menu bar (kinda
> like where tray icons go on macOS).
>
>
> --
> Dave Page
> Blog: http://pgsnake.blogspot.com
> Twitter: @pgsnake
>
> EnterpriseDB UK: http://www.enterprisedb.com
> The Enterprise PostgreSQL Company
>

Re: Showstopper desktop runtime issue

[Joao De Almeida Pereira]

Hello Hackers,
Here is our take on the 3 options present:
1) QtWebkit and QtWebEngine

Pros:
- Webkit is a mature technology that as been around for some time
- Apparently the Fork of Webkit we were using as been catching up
Cons:
- We are using a Fork
- It is to far behind of the today browsers and the competition keep
evolving making it even harder for them to catch up
- Problems with the environment from the past, lagginess, lack of support

2) Rework runtime to find a running PGAdmin and attach to it

Pros:
- We already made the big move from Webkit to this version
- Add smarts to the application to do not run itself if it is already
running, or kill the currently running application

Cons:
- More C++ code thrown around
- Add more dependency on our QT application
- Have specific code for Gnome, not even a OS version but a window manager

3) Electron

Pros:
- Full Javascript runtime, no more C++ code to handle
- Self packaging for all the environments
Cons:
- No fully supported on Linux based OS with old libstdc++, (This might
solve the problem: https://github.com/mattermost/desktop/issues/312)
- CPU/RAM high utilization (Found a issues around and the majority of them
have solutions like CSS animations running or settings on the created
windows)
- Electron does not support tabs by default, only HTML tabs

After combing through these options with some pros and cons  here are my
thoughts:
1) This options was always cluncky from the beginning, the fact that we are
working with a browser that is currently 26k commits behind Webkit:master
is concerning. So I do not believe this is a viable option. As a personal
opinion is we need to release fast, then we should go with what we have now
and not go back to this option.
2) This option is not very appealing to me, because we would be pilling
code into the QT portion of the application, that I hope we remove in the
future, that currently is untested to solve a problem caused by a Window
Manager..
I would be more in favor of creating a Application Indicator that would
support 2 actions, Kill running pgAdmins and launch the browser to access
them. There is a lot of interesting websites that talk about this, and how
to develop. I had to download one to have a docker indicator. But as I
said in a previous thread, I believe that this should be a 3rd party
application and not a first class citizen on pgAdmin, as the majority of
the ones that I found are.
3) Electron can to play, first when we though about removing QTWebkit,
because it always packs a browser with it and also and there is a very big
Open Source Community being it. It came back again when when we saw that we
were ready to ditch the browser. (As a personal note I dislike applications
that I install in my machine and pop browser open to do anything, this is a
personal issue that I would like to see if it was shared by others that was
why we decided to park Electron for a second time. Because if our users are
ok with it there is no need to add extra complexity to the product)
The most interesting thing about the cons that I found in Electron there is
an issue open for them and a bunch of different solutions to try to handle
it, or even advises on how to handle it. This option also allow us to focus
less on browser compatibility as the main target of our development will
become Chromium which is the base of browsers like UC Browser, Vivaldi,
Opera, Google Chrome.

As you could see from my long long email, I am biased towards options 3,
not because I started the push for it, but because I believe it is the
right step for this product. This being said, I think that releasing now
with this current issue is Ok as a first step to somewhere.

Thanks
Joao

On Thu, Mar 22, 2018 at 6:48 AM Murtuza Zabuawala <
murtuza.zabuaw...@enterprisedb.com> wrote:

> On Thu, Mar 22, 2018 at 3:19 PM, Dave Page  wrote:
>
>> All,
>>
>> As you know, the 3.0 release is currently on hold as we discovered late
>> yesterday that the re-vamped desktop runtime will not run on Gnome 3.26 and
>> later. This is because the GTK project, and later Gnome, have removed
>> support for the System Tray on which the new runtime relies.
>>
>> They have replaced it with a notification mechanism, however this doesn't
>> really meet our needs as what we want is a place (the tray icon) to attach
>> a menu to control the pgAdmin server; we don't really use notifications as
>> such.
>>
>> I see a number of possible ways around this:
>>
>> 1) Return to the previous runtime. I think this is at best a short-term
>> solution, as the re-visited Annulen version of the QtWebKit seems to be
>> getting little attention at the moment, and this would re-introduce many
>> known bugs caused by WebKit.
>>
> ​I would not prefer going back after seeing QtWebkit & QtWebEngine issues
> in the past.​
>
>
>>
>> 2) Re-work the current runtime code to remove the tray icon, and utilise
>> desktop/start menu items to signal the running instance to show 

Re: Connection Refused Error

[Joao De Almeida Pereira]

Hello Rahul,
Did you change the /etc/postgresql/10/main/pg_hba.conf  file configuration
to allow connections? if not take a look at
https://www.thegeekstuff.com/2014/02/enable-remote-postgresql-connection/ this
should guide you but basically you need to change the METHOD to `trust` on
the 127.0.0.1 address and eventually of the ::1 as well.
If that didn't help let us know

Thanks
Joao

On Wed, Mar 21, 2018 at 3:35 PM Rahul Soshte 
wrote:

> I had installed pgadmin4 from git Source repo.The First time I Installed I
> was able to create a Server.But when I restarted my PC.I got this error.
>
> Unable to connect to server:
>
> could not connect to server: Connection refused
> Is the server running on host "127.0.0.1" and accepting
> TCP/IP connections on port 5432?
>
> I tried Installing everthing Again but got the same error.I also changed
> postgresql.conf file from
> listen_address="localhost"
> to listen_address="*"
> but it still didnt work.
>
> What do I do?
>

Re: v3.0 release on hold

[Joao De Almeida Pereira]

Hi Dave,
I think that this Gnome issue should be addressed after the release of 3.0.
We should create a bug and see the best way to address it after. This is my
proposal, because I am not 100% sure where this application indicator
should live.
If we search the web for application indicators we can see that usually
these are developed by 3rd parties and is are present at the applications
repository, so this might be a side project that need to be undertaken, by
someone in the community or not, but it is not something that should live
inside our repository as it is not really part of the code source and it is
more like a Hack for Gnome.

The other issues I believe need to be addressed, specially if they are
crippling to the application like when you click it does not start, but if
they are edge cases, we can always release this week and have a new release
in 2 weeks or something with more fixes to these edge case problems.

I understand that the process of release at this point is a bit cumbersome
and take a lot of time, but  if we can get more tests around the new and
old feature we can have more confidence in our code and as a result of that
we can automate some of the steps in order to generate binaries more
frequently.

Thanks
Joao

On Wed, Mar 21, 2018 at 12:54 PM Dave Page <dp...@pgadmin.org> wrote:

> Hi
>
> On Wed, Mar 21, 2018 at 4:22 PM, Joao De Almeida Pereira <
> jdealmeidapere...@pivotal.io> wrote:
>
>> Hello Dave,
>> For number 1:
>> https://blogs.gnome.org/aday/2017/08/31/status-icons-and-gnome/
>> We might need to build something like:
>> https://itsfoss.com/best-indicator-applets-ubuntu/
>> Not sure if it is wise to do it in such a short notice.
>>
>
> Yeah, the problem with the suggested solutions is that they rely on 3rd
> party extensions that aren't "real" packages for the OS, so we can't just
> add a dependency on them. Unfortunately I think this is going to cause
> quite a bit of work to get 3.0 back on track.
>
>
>>
>> Thanks
>> Joao
>>
>> On Wed, Mar 21, 2018 at 11:38 AM Khushboo Vashi <
>> khushboo.va...@enterprisedb.com> wrote:
>>
>>>
>>>
>>> On 21 Mar 2018 21:05, "Dave Page" <dp...@pgadmin.org> wrote:
>>>
>>> We've run into a number of unexpected issues with the v3.0 release that
>>> I think we need to resolve before moving forwards. For the time being, only
>>> patches critical to fix these issues should be committed.
>>>
>>> I'll try to look at 1, though I do have another deadline I need to meet.
>>> Akshay, can you look at 2 please?
>>> Fahar is already looking at 3.
>>> Khushboo, can you look at 4 please?
>>>
>>> Sure.
>>>
>>>
>>> Thanks all.
>>>
>>> 1) There is no longer a system tray in Gnome 3.26 and later, and thus
>>> the runtime won't initialise in Fedora 27 and later. We need an alternative
>>> for this, either a tray replacement that the RPM can depend on, or better
>>> yet, support whatever it is Gnome expect such apps to use these days.
>>>
>>> 2) Starting a second instance of the app bundle on Mac doesn't always
>>> open a new pgAdmin window as it should. It works fine in the debugger, or
>>> if you start the app with a command like: "/Applications/pgAdmin\
>>> 4.app/Contents/MacOS/pgAdmin4". It doesn't work if you double-click the
>>> appbundle or use a command like "open /Applications/pgAdmin\ 4.app"
>>>
>>> 3) Fahar saw a crash on Windows 7. I couldn't reproduce this on my copy,
>>> but apparently his is a fresh installation.
>>>
>>> 4) On my Windows 7 machine, after running a backup I get no status
>>> window, and see the following in the logs:
>>>
>>> Traceback (most recent call last):
>>>   File "C:\Program Files (x86)\pgAdmin
>>> 4\v3\venv\Lib\site-packages\werkzeug\serving.py", line 209, in run_wsgi
>>> execute(self.server.app)
>>>   File "C:\Program Files (x86)\pgAdmin
>>> 4\v3\venv\Lib\site-packages\werkzeug\serving.py", line 197, in execute
>>> application_iter = app(environ, start_response)
>>>   File "C:\Program Files (x86)\pgAdmin
>>> 4\v3\venv\Lib\site-packages\flask\app.py", line 1997, in __call__
>>> return self.wsgi_app(environ, start_response)
>>>   File "C:\Program Files (x86)\pgAdmin
>>> 4\v3\venv\Lib\site-packages\flask\app.py", line 1985, in wsgi_app
>>> response = self.handle_exception(e)
>>>   File "C:\Program Files (x86)\pgAdmin
>>> 4\v3\venv\Lib\site-pa

Re: Experiencing issues

[Joao De Almeida Pereira]

Another thing when I do this configuration:

DEFAULT_SERVER = '127.0.0.1'
SESSION_COOKIE_DOMAIN = 'localhost'
COOKIE_DEFAULT_DOMAIN = 'localhost'

I get the following exception:
builtins.ValueError

ValueError: Setting 'domain' for a cookie on a server running localy (ex:
localhost) is not supportted by complying browsers. You should have
something like: '127.0.0.1 localhost dev.localhost' on your hosts file and
then point your server to run on 'dev.localhost' and also set 'domain' for
'dev.localhost'
And if you follow the instructions and change the hosts file it allows you
to start the application but when you try to open a database server you
will get the 428 error
Thanks
Joao


On Wed, Mar 21, 2018 at 12:01 PM Dave Page <dp...@pgadmin.org> wrote:

> On Wed, Mar 21, 2018 at 3:57 PM, Joao De Almeida Pereira <
> jdealmeidapere...@pivotal.io> wrote:
>
>> Sorry I did not understand what you said.
>> This configuration:
>>
>> DEFAULT_SERVER = '0.0.0.0'
>> SESSION_COOKIE_DOMAIN = DEFAULT_SERVER
>> COOKIE_DEFAULT_DOMAIN = DEFAULT_SERVER
>>
>> If the application lives in the domain pgadmin.somedomain.com do I need
>> to have in config_local:
>> DEFAULT_SERVER = '0.0.0.0'
>> SESSION_COOKIE_DOMAIN = 'pgadmin.somedomain.com'
>> COOKIE_DEFAULT_DOMAIN = 'pgadmin.somedomain.com'
>> ?
>>
>> Does this mean that if for some reason I have a second domain like
>> pgadmin.somedomain2.com that I want to use I cannot?
>>
>> The issue of 127.0.0.1 to localhost is very cumbersome, and somehow we
>> should be able to disable this, because when we are developing doesn't make
>> sense to not being able to use localhost and 127.0.0.1
>>
>
> +1. I didn't realise we'd added this restriction when I tested the patch.
>
> Perhaps a better approach would be to leave the default cookie handling as
> it was, and just expose the domain and path via config options that the
> user can set if appropriate for their installation.
>
>
>
>>
>> Thanks
>> Joao
>>
>> On Wed, Mar 21, 2018 at 11:01 AM Khushboo Vashi <
>> khushboo.va...@enterprisedb.com> wrote:
>>
>>> On Wed, Mar 21, 2018 at 8:27 PM, Joao De Almeida Pereira <
>>> jdealmeidapere...@pivotal.io> wrote:
>>>
>>>> So what you are saying is that if I have a server, I need to do
>>>> DEFAULT_SERVER=0.0.0.0 and then set the real domain on the COOKIE domain?
>>>>
>>>> No I am saying, whatever you set as a DEFAULT_SERVER,  the app can be
>>> accessible with that server.
>>> As, we have explicitly set  DOMAIN in the cookie setting.
>>>
>>>> On Wed, Mar 21, 2018 at 10:55 AM Khushboo Vashi <
>>>> khushboo.va...@enterprisedb.com> wrote:
>>>>
>>>>> On Wed, Mar 21, 2018 at 8:10 PM, Joao De Almeida Pereira <
>>>>> jdealmeidapere...@pivotal.io> wrote:
>>>>>
>>>>>> Ok Murtuza you are right,
>>>>>> Now my question is I have the default server to 127.0.0.1 and I want
>>>>>> to access it using localhost as well. How can I do this?
>>>>>>
>>>>>> No, you can't.
>>>>> Domain based cookie will work for that domain and it's sub-domains.
>>>>>
>>>>>> On Wed, Mar 21, 2018 at 10:39 AM Khushboo Vashi <
>>>>>> khushboo.va...@enterprisedb.com> wrote:
>>>>>>
>>>>>>>
>>>>>>>
>>>>>>> On 21 Mar 2018 20:01, "Joao De Almeida Pereira" <
>>>>>>> jdealmeidapere...@pivotal.io> wrote:
>>>>>>>
>>>>>>> I tried that but still nothing. When i check in the inspector for
>>>>>>> cookies I have none
>>>>>>>
>>>>>>> Share your config_local file.
>>>>>>>
>>>>>>> On Wed, Mar 21, 2018 at 10:30 AM Murtuza Zabuawala <
>>>>>>> murtuza.zabuaw...@enterprisedb.com> wrote:
>>>>>>>
>>>>>>>> Yes, that's cookie related issue (RM#3197), To fix that I added
>>>>>>>> below in my config_local.py and it started working again,
>>>>>>>>
>>>>>>>> DEFAULT_SERVER = '0.0.0.0'
>>>>>>>> COOKIE_DEFAULT_DOMAIN = SESSION_COOKIE_DOMAIN = DEFAULT_SERVER
>>>>>>>>
>>>>>>>> Clear your browser cookies and server side sessions.
>>>>>>>>
>&g

Re: v3.0 release on hold

[Joao De Almeida Pereira]

Hello Dave,
For number 1:
https://blogs.gnome.org/aday/2017/08/31/status-icons-and-gnome/
We might need to build something like:
https://itsfoss.com/best-indicator-applets-ubuntu/
Not sure if it is wise to do it in such a short notice.

Thanks
Joao

On Wed, Mar 21, 2018 at 11:38 AM Khushboo Vashi <
khushboo.va...@enterprisedb.com> wrote:

>
>
> On 21 Mar 2018 21:05, "Dave Page"  wrote:
>
> We've run into a number of unexpected issues with the v3.0 release that I
> think we need to resolve before moving forwards. For the time being, only
> patches critical to fix these issues should be committed.
>
> I'll try to look at 1, though I do have another deadline I need to meet.
> Akshay, can you look at 2 please?
> Fahar is already looking at 3.
> Khushboo, can you look at 4 please?
>
> Sure.
>
>
> Thanks all.
>
> 1) There is no longer a system tray in Gnome 3.26 and later, and thus the
> runtime won't initialise in Fedora 27 and later. We need an alternative for
> this, either a tray replacement that the RPM can depend on, or better yet,
> support whatever it is Gnome expect such apps to use these days.
>
> 2) Starting a second instance of the app bundle on Mac doesn't always open
> a new pgAdmin window as it should. It works fine in the debugger, or if you
> start the app with a command like: "/Applications/pgAdmin\
> 4.app/Contents/MacOS/pgAdmin4". It doesn't work if you double-click the
> appbundle or use a command like "open /Applications/pgAdmin\ 4.app"
>
> 3) Fahar saw a crash on Windows 7. I couldn't reproduce this on my copy,
> but apparently his is a fresh installation.
>
> 4) On my Windows 7 machine, after running a backup I get no status window,
> and see the following in the logs:
>
> Traceback (most recent call last):
>   File "C:\Program Files (x86)\pgAdmin
> 4\v3\venv\Lib\site-packages\werkzeug\serving.py", line 209, in run_wsgi
> execute(self.server.app)
>   File "C:\Program Files (x86)\pgAdmin
> 4\v3\venv\Lib\site-packages\werkzeug\serving.py", line 197, in execute
> application_iter = app(environ, start_response)
>   File "C:\Program Files (x86)\pgAdmin
> 4\v3\venv\Lib\site-packages\flask\app.py", line 1997, in __call__
> return self.wsgi_app(environ, start_response)
>   File "C:\Program Files (x86)\pgAdmin
> 4\v3\venv\Lib\site-packages\flask\app.py", line 1985, in wsgi_app
> response = self.handle_exception(e)
>   File "C:\Program Files (x86)\pgAdmin
> 4\v3\venv\Lib\site-packages\flask\app.py", line 1540, in handle_exception
> reraise(exc_type, exc_value, tb)
>   File "C:\Program Files (x86)\pgAdmin
> 4\v3\venv\Lib\site-packages\flask\app.py", line 1982, in wsgi_app
> response = self.full_dispatch_request()
>   File "C:\Program Files (x86)\pgAdmin
> 4\v3\venv\Lib\site-packages\flask\app.py", line 1614, in
> full_dispatch_request
> rv = self.handle_user_exception(e)
>   File "C:\Program Files (x86)\pgAdmin
> 4\v3\venv\Lib\site-packages\flask\app.py", line 1517, in
> handle_user_exception
> reraise(exc_type, exc_value, tb)
>   File "C:\Program Files (x86)\pgAdmin
> 4\v3\venv\Lib\site-packages\flask\app.py", line 1612, in
> full_dispatch_request
> rv = self.dispatch_request()
>   File "C:\Program Files (x86)\pgAdmin
> 4\v3\venv\Lib\site-packages\flask\app.py", line 1598, in dispatch_request
> return self.view_functions[rule.endpoint](**req.view_args)
>   File "C:\Program Files (x86)\pgAdmin
> 4\v3\venv\Lib\site-packages\flask_login.py", line 792, in decorated_view
> return func(*args, **kwargs)
>   File "C:\Program Files (x86)\pgAdmin
> 4\v3\web\pgadmin\misc\bgprocess\__init__.py", line 62, in index
> return make_response(response=BatchProcess.list())
>   File "C:\Program Files (x86)\pgAdmin
> 4\v3\web\pgadmin\misc\bgprocess\processes.py", line 584, in list
> details = desc.details(p.command, args)
>   File "C:\Program Files (x86)\pgAdmin
> 4\v3\web\pgadmin\tools\backup\__init__.py", line 190, in details
> res += html.safe_str(cmd + self.cmd)
> AttributeError: 'BackupMessage' object has no attribute 'cmd'
>
>
>
> --
> Dave Page
> Blog: http://pgsnake.blogspot.com
> Twitter: @pgsnake
>
> EnterpriseDB UK: http://www.enterprisedb.com
> The Enterprise PostgreSQL Company
>
>
>

Re: Experiencing issues

[Joao De Almeida Pereira]

So what you are saying is that if I have a server, I need to do
DEFAULT_SERVER=0.0.0.0 and then set the real domain on the COOKIE domain?

On Wed, Mar 21, 2018 at 10:55 AM Khushboo Vashi <
khushboo.va...@enterprisedb.com> wrote:

> On Wed, Mar 21, 2018 at 8:10 PM, Joao De Almeida Pereira <
> jdealmeidapere...@pivotal.io> wrote:
>
>> Ok Murtuza you are right,
>> Now my question is I have the default server to 127.0.0.1 and I want to
>> access it using localhost as well. How can I do this?
>>
>> No, you can't.
> Domain based cookie will work for that domain and it's sub-domains.
>
>> On Wed, Mar 21, 2018 at 10:39 AM Khushboo Vashi <
>> khushboo.va...@enterprisedb.com> wrote:
>>
>>>
>>>
>>> On 21 Mar 2018 20:01, "Joao De Almeida Pereira" <
>>> jdealmeidapere...@pivotal.io> wrote:
>>>
>>> I tried that but still nothing. When i check in the inspector for
>>> cookies I have none
>>>
>>> Share your config_local file.
>>>
>>> On Wed, Mar 21, 2018 at 10:30 AM Murtuza Zabuawala <
>>> murtuza.zabuaw...@enterprisedb.com> wrote:
>>>
>>>> Yes, that's cookie related issue (RM#3197), To fix that I added below
>>>> in my config_local.py and it started working again,
>>>>
>>>> DEFAULT_SERVER = '0.0.0.0'
>>>> COOKIE_DEFAULT_DOMAIN = SESSION_COOKIE_DOMAIN = DEFAULT_SERVER
>>>>
>>>> Clear your browser cookies and server side sessions.
>>>>
>>>>
>>>> --
>>>> Regards,
>>>> Murtuza Zabuawala
>>>> EnterpriseDB: http://www.enterprisedb.com
>>>> The Enterprise PostgreSQL Company
>>>>
>>>>
>>>> On Wed, Mar 21, 2018 at 7:55 PM, Joao De Almeida Pereira <
>>>> jdealmeidapere...@pivotal.io> wrote:
>>>>
>>>>> Where can I find information about that?
>>>>>
>>>>> On Wed, Mar 21, 2018 at 10:16 AM Khushboo Vashi <
>>>>> khushboo.va...@enterprisedb.com> wrote:
>>>>>
>>>>>>
>>>>>>
>>>>>> On 21 Mar 2018 19:41, "Joao De Almeida Pereira" <
>>>>>> jdealmeidapere...@pivotal.io> wrote:
>>>>>>
>>>>>> Hello Hackers,
>>>>>> Can anyone use the current master branch?
>>>>>> When I try to open a server I get a 428. Is that only me?
>>>>>>
>>>>>> May be because of cookie changes.
>>>>>> Check your config.py and config_local.py if you have done changes
>>>>>> related to DEFAULT_SERVER in your config_local.py then you need to change
>>>>>> other 2 cookie related variables also.
>>>>>>
>>>>>> Thanks
>>>>>> Joao
>>>>>>
>>>>>>
>>>>>>
>>>>
>>>

Re: Experiencing issues

[Joao De Almeida Pereira]

Ok Murtuza you are right,
Now my question is I have the default server to 127.0.0.1 and I want to
access it using localhost as well. How can I do this?

On Wed, Mar 21, 2018 at 10:39 AM Khushboo Vashi <
khushboo.va...@enterprisedb.com> wrote:

>
>
> On 21 Mar 2018 20:01, "Joao De Almeida Pereira" <
> jdealmeidapere...@pivotal.io> wrote:
>
> I tried that but still nothing. When i check in the inspector for cookies
> I have none
>
> Share your config_local file.
>
> On Wed, Mar 21, 2018 at 10:30 AM Murtuza Zabuawala <
> murtuza.zabuaw...@enterprisedb.com> wrote:
>
>> Yes, that's cookie related issue (RM#3197), To fix that I added below in
>> my config_local.py and it started working again,
>>
>> DEFAULT_SERVER = '0.0.0.0'
>> COOKIE_DEFAULT_DOMAIN = SESSION_COOKIE_DOMAIN = DEFAULT_SERVER
>>
>> Clear your browser cookies and server side sessions.
>>
>>
>> --
>> Regards,
>> Murtuza Zabuawala
>> EnterpriseDB: http://www.enterprisedb.com
>> The Enterprise PostgreSQL Company
>>
>>
>> On Wed, Mar 21, 2018 at 7:55 PM, Joao De Almeida Pereira <
>> jdealmeidapere...@pivotal.io> wrote:
>>
>>> Where can I find information about that?
>>>
>>> On Wed, Mar 21, 2018 at 10:16 AM Khushboo Vashi <
>>> khushboo.va...@enterprisedb.com> wrote:
>>>
>>>>
>>>>
>>>> On 21 Mar 2018 19:41, "Joao De Almeida Pereira" <
>>>> jdealmeidapere...@pivotal.io> wrote:
>>>>
>>>> Hello Hackers,
>>>> Can anyone use the current master branch?
>>>> When I try to open a server I get a 428. Is that only me?
>>>>
>>>> May be because of cookie changes.
>>>> Check your config.py and config_local.py if you have done changes
>>>> related to DEFAULT_SERVER in your config_local.py then you need to change
>>>> other 2 cookie related variables also.
>>>>
>>>> Thanks
>>>> Joao
>>>>
>>>>
>>>>
>>
>

Re: Experiencing issues

[Joao De Almeida Pereira]

I tried that but still nothing. When i check in the inspector for cookies I
have none

On Wed, Mar 21, 2018 at 10:30 AM Murtuza Zabuawala <
murtuza.zabuaw...@enterprisedb.com> wrote:

> Yes, that's cookie related issue (RM#3197), To fix that I added below in
> my config_local.py and it started working again,
>
> DEFAULT_SERVER = '0.0.0.0'
> COOKIE_DEFAULT_DOMAIN = SESSION_COOKIE_DOMAIN = DEFAULT_SERVER
>
> Clear your browser cookies and server side sessions.
>
>
> --
> Regards,
> Murtuza Zabuawala
> EnterpriseDB: http://www.enterprisedb.com
> The Enterprise PostgreSQL Company
>
>
> On Wed, Mar 21, 2018 at 7:55 PM, Joao De Almeida Pereira <
> jdealmeidapere...@pivotal.io> wrote:
>
>> Where can I find information about that?
>>
>> On Wed, Mar 21, 2018 at 10:16 AM Khushboo Vashi <
>> khushboo.va...@enterprisedb.com> wrote:
>>
>>>
>>>
>>> On 21 Mar 2018 19:41, "Joao De Almeida Pereira" <
>>> jdealmeidapere...@pivotal.io> wrote:
>>>
>>> Hello Hackers,
>>> Can anyone use the current master branch?
>>> When I try to open a server I get a 428. Is that only me?
>>>
>>> May be because of cookie changes.
>>> Check your config.py and config_local.py if you have done changes
>>> related to DEFAULT_SERVER in your config_local.py then you need to change
>>> other 2 cookie related variables also.
>>>
>>> Thanks
>>> Joao
>>>
>>>
>>>
>

Re: Experiencing issues

[Joao De Almeida Pereira]

Where can I find information about that?

On Wed, Mar 21, 2018 at 10:16 AM Khushboo Vashi <
khushboo.va...@enterprisedb.com> wrote:

>
>
> On 21 Mar 2018 19:41, "Joao De Almeida Pereira" <
> jdealmeidapere...@pivotal.io> wrote:
>
> Hello Hackers,
> Can anyone use the current master branch?
> When I try to open a server I get a 428. Is that only me?
>
> May be because of cookie changes.
> Check your config.py and config_local.py if you have done changes related
> to DEFAULT_SERVER in your config_local.py then you need to change other 2
> cookie related variables also.
>
> Thanks
> Joao
>
>
>

Experiencing issues

[Joao De Almeida Pereira]

Hello Hackers,
Can anyone use the current master branch?
When I try to open a server I get a 428. Is that only me?

Thanks
Joao

[pgadmin4][patch] Update karma to 2.0 #3222

[Joao De Almeida Pereira]

Hi Hackers,
Attached you can find the patch that updates karma to 2.0 and also other
plugins from karma.

The only plugins that are not updated are the jasmine ones because we are
waiting on the support for jasmine 3.0 on karma-jasmine

Thanks
Joao


karma-plugins-update.diff
Description: Binary data

Re: pgAdmin 4 commit: Refactor server dialogue validation for better unit t

[Joao De Almeida Pereira]

Hi Khushboo,
Good catch, attached you can find the fix for the problem.

Thanks
Victoria & Joao

On Wed, Mar 14, 2018 at 10:37 AM Khushboo Vashi <
khushboo.va...@enterprisedb.com> wrote:

> Hi Joao/Victoria,
>
> I am getting an error while editing the server properties. Please refer
> the attached screen-shot for the same.
>
> Thanks,
> Khushboo
>
> On Wed, Mar 14, 2018 at 12:17 AM, Dave Page  wrote:
>
>> Refactor server dialogue validation for better unit testing.
>>
>> Victoria & Joao @ Pivotal.
>>
>> Branch
>> --
>> master
>>
>> Details
>> ---
>>
>> https://git.postgresql.org/gitweb?p=pgadmin4.git;a=commitdiff;h=6b03cb78af607b04c75de44d635bf201babb4c5a
>> Author: Victoria Henry 
>>
>> Modified Files
>> --
>> web/package.json   |   3 +-
>> .../server_groups/servers/static/js/server.js  | 113
>> ++---
>> web/pgadmin/static/bundle/browser.js   |   2 +-
>> web/pgadmin/static/js/browser/index.js |  10 ++
>> .../static/js/browser/server_groups/index.js   |  10 ++
>> .../server_groups/servers/databases/index.js   |  10 ++
>> .../js/browser/server_groups/servers/index.js  |  11 ++
>> .../server_groups/servers/model_validation.js  | 104
>> +++
>> .../server_groups/servers/model_validation_spec.js | 101
>> ++
>> web/yarn.lock  |  46 -
>> 10 files changed, 299 insertions(+), 111 deletions(-)
>>
>>
>
diff --git a/web/pgadmin/static/js/browser/server_groups/servers/model_validation.js b/web/pgadmin/static/js/browser/server_groups/servers/model_validation.js
index 7ab129ba..948af3d4 100644
--- a/web/pgadmin/static/js/browser/server_groups/servers/model_validation.js
+++ b/web/pgadmin/static/js/browser/server_groups/servers/model_validation.js
@@ -99,6 +99,6 @@ export class ModelValidation {
   }
 
   static isEmptyString(string) {
-return _.isUndefined(string) || _.isNull(string) || string.trim() === '';
+return _.isUndefined(string) || _.isNull(string) || String(string).trim() === '';
   }
 }
diff --git a/web/regression/javascript/browser/server_groups/servers/model_validation_spec.js b/web/regression/javascript/browser/server_groups/servers/model_validation_spec.js
index a05cd455..560d8b60 100644
--- a/web/regression/javascript/browser/server_groups/servers/model_validation_spec.js
+++ b/web/regression/javascript/browser/server_groups/servers/model_validation_spec.js
@@ -31,7 +31,7 @@ describe('Server#ModelValidation', () => {
 model.isNew.and.returnValue(true);
 model.allValues['name'] = 'some name';
 model.allValues['username'] = 'some username';
-model.allValues['port'] = 'some port';
+model.allValues['port'] = 12345;
   });
 
   describe('No service id', () => {

Re: pgAdmin 4 commit: Fix unicode handling in the external process tools an

[Joao De Almeida Pereira]

Hi Dave, Khushboo

Looks like there is a linting issue with this commit:

Successfully installed pycodestyle-2.3.1
 

/tmp/build/4a5630c2/pgadmin-master/web /tmp/build/4a5630c2
 

./pgadmin/misc/bgprocess/processes.py:186: [E225] missing whitespace
around operator
 

./pgadmin/tools/backup/__init__.py:113: [E303] too many blank lines (2)
 

1   E225 missing whitespace around operator
 

1   E303 too many blank lines (2)
 

2


Best Regards,
Victoria & Joao

On Tue, Mar 13, 2018 at 4:45 PM Dave Page  wrote:

> Fix unicode handling in the external process tools and show the complete
> command in the process viewer. Fixes #2963. Fixes #3157.
>
> Branch
> --
> master
>
> Details
> ---
>
> https://git.postgresql.org/gitweb?p=pgadmin4.git;a=commitdiff;h=802269910c37a52aa57479bbcc1d8d078f6a88e1
> Author: Khushboo Vashi 
>
> Modified Files
> --
> web/pgadmin/misc/bgprocess/processes.py   | 36
> +--
> web/pgadmin/misc/bgprocess/static/js/bgprocess.js |  6 ++--
> web/pgadmin/tools/backup/__init__.py  |  5 ++--
> 3 files changed, 38 insertions(+), 9 deletions(-)
>
>

Re: [pgAdmin4][RM#3140] Add service parameter

[Joao De Almeida Pereira]

Hi Dave and Murtuza,

Regarding this patch we refactored the Javascript code so that is lives in
a different file and added some tests.

Also we found an issue with karma-jasmine that does not allow us to use
jasmine 3.1 yet. You can find attached a patch that reverts that commit.

Thanks
Victoria && Joao

On Mon, Mar 12, 2018 at 4:46 PM Dave Page  wrote:

> Thanks, patch applied!
>
> On Mon, Mar 12, 2018 at 3:31 AM, Murtuza Zabuawala <
> murtuza.zabuaw...@enterprisedb.com> wrote:
>
>> Hi Dave,
>>
>> PFA updated patch.
>>
>> --
>> Regards,
>> Murtuza Zabuawala
>> EnterpriseDB: http://www.enterprisedb.com
>> The Enterprise PostgreSQL Company
>>
>>
>> On Fri, Mar 9, 2018 at 9:29 PM, Murtuza Zabuawala <
>> murtuza.zabuaw...@enterprisedb.com> wrote:
>>
>>> Hi Dave,
>>>
>>> I'll change the name and send you updated patch.
>>>
>>>
>>> On Fri, Mar 9, 2018 at 9:25 PM, Dave Page  wrote:
>>>
 HI

 On Fri, Mar 9, 2018 at 11:47 AM, Murtuza Zabuawala <
 murtuza.zabuaw...@enterprisedb.com> wrote:

> Hi,
>
> PFA patch to add service parameter in server dialog.
> - Docs updated
> - Test case added for Service ID parameter
>
> Please note,
> I have extracted Connection class and Server manager class from our
> own custom Psycopg2 driver module.
>
> Patch also covers RM#3120
>

  This patch seems a little confused. The "Service" and "Service ID"
 fields from pgAdmin 3 are very different things. The Redmine ticket seems
 to be asking for the Service field (the pg_service.conf service name),
 *not* Service ID (the operating system's service ID, used to start/stop the
 database server service).

 --
 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
>
diff --git a/web/package.json b/web/package.json
index ad0f3e16..66684fc6 100644
--- a/web/package.json
+++ b/web/package.json
@@ -69,6 +69,7 @@
 "hard-source-webpack-plugin": "^0.4.9",
 "immutability-helper": "^2.2.0",
 "imports-loader": "^0.7.1",
+"ip-address": "^5.8.9",
 "jquery": "1.11.2",
 "jquery-contextmenu": "^2.5.0",
 "jquery-ui": "^1.12.1",
diff --git a/web/pgadmin/browser/server_groups/servers/static/js/server.js b/web/pgadmin/browser/server_groups/servers/static/js/server.js
index 53be2255..302fe458 100644
--- a/web/pgadmin/browser/server_groups/servers/static/js/server.js
+++ b/web/pgadmin/browser/server_groups/servers/static/js/server.js
@@ -2,10 +2,13 @@ define('pgadmin.node.server', [
   'sources/gettext', 'sources/url_for', 'jquery', 'underscore', 'backbone',
   'underscore.string', 'sources/pgadmin', 'pgadmin.browser',
   'pgadmin.server.supported_servers', 'pgadmin.user_management.current_user',
-  'pgadmin.alertifyjs', 'pgadmin.backform', 'pgadmin.browser.server.privilege',
+  'pgadmin.alertifyjs', 'pgadmin.backform',
+  'sources/browser/server_groups/servers/model_validation',
+  'pgadmin.browser.server.privilege',
 ], function(
   gettext, url_for, $, _, Backbone, S, pgAdmin, pgBrowser,
-  supported_servers, current_user, Alertify, Backform
+  supported_servers, current_user, Alertify, Backform,
+  modelValidation
 ) {
 
   if (!pgBrowser.Nodes['server']) {
@@ -848,110 +851,8 @@ define('pgadmin.node.server', [
   group: gettext('Connection'),
 }],
 validate: function() {
-  var err = {},
-errmsg,
-self = this;
-
-  var service_id = this.get('service');
-
-  var check_for_empty = function(id, msg) {
-var v = self.get(id);
-if (
-  _.isUndefined(v) || v === null || String(v).replace(/^\s+|\s+$/g, '') == ''
-) {
-  err[id] = msg;
-  errmsg = errmsg || msg;
-  return true;
-} else {
-  self.errorModel.unset(id);
-  return false;
-}
-  };
-  var check_for_valid_ipv6 = function(val){
-// Regular expression for validating IPv6 address formats
-var exps = ['^\s*((([0-9A-Fa-f]{1,4}:){7}([0-9A-Fa-f]{1,4}|:))|',
-  '(([0-9A-Fa-f]{1,4}:){6}(:[0-9A-Fa-f]{1,4}|((25[0-5]|',
-  '2[0-4]\d|1\d\d|[1-9]?\d)(\.(25[0-5]|2[0-4]\d|1\d\d|[1-9]?\d)){3})|:))|',
-  '(([0-9A-Fa-f]{1,4}:){5}(((:[0-9A-Fa-f]{1,4}){1,2})|',
-  ':((25[0-5]|2[0-4]\d|1\d\d|[1-9]?\d)(\.(25[0-5]|2[0-4]\d|1\d\d|[1-9]?\d)){3})|:))|',
-  '(([0-9A-Fa-f]{1,4}:){4}(((:[0-9A-Fa-f]{1,4}){1,3})|((:[0-9A-Fa-f]{1,4})?:((25[0-5]|',
-  '2[0-4]\d|1\d\d|[1-9]?\d)(\.(25[0-5]|2[0-4]\d|1\d\d|[1-9]?\d)){3}))|:))|',
-  

Re: pgAdmin 4 commit: Support for external tables in GPDB. Fixes #3168

[Joao De Almeida Pereira]

Hello Murtuza,
Yes I think it makes sense

Thanks
Joao

On Mon, Mar 12, 2018 at 10:40 AM Murtuza Zabuawala <
murtuza.zabuaw...@enterprisedb.com> wrote:

> Hi Joao,
>
> Can we make tests to skip if db is not greenplum?
> Like we are doing for resource group
> "..web/pgadmin/browser/server_groups/servers/resource_groups/tests".
>
>
> On Mon, Mar 12, 2018 at 7:52 PM, Murtuza Zabuawala <
> murtuza.zabuaw...@enterprisedb.com> wrote:
>
>> Hi Dave,
>>
>> Joao's patch is pending, would you please do the needful?
>> I also encounter similar issue on Windows while running tests.
>>
>> --
>> Regards,
>> Murtuza Zabuawala
>> EnterpriseDB: http://www.enterprisedb.com
>> The Enterprise PostgreSQL Company
>>
>>
>> On Tue, Mar 6, 2018 at 8:25 PM, Joao De Almeida Pereira <
>> jdealmeidapere...@pivotal.io> wrote:
>>
>>> Hello Neel,
>>>
>>> You can find attached the corrections of the path's needed for windows.
>>> The fix should correct TestExternalTablesView and TestTemplateCreate
>>> but for the ChangePasswordTestCase I need more information to help you out.
>>> We need to understand what is the response that the endpoint
>>> /user_management/user is returning.
>>>
>>> Thanks
>>> Joao
>>>
>>> On Tue, Mar 6, 2018 at 2:29 AM Neel Patel <neel.pa...@enterprisedb.com>
>>> wrote:
>>>
>>>> Hi Joao,
>>>>
>>>> I ran the testsuite in windows 10 with Python 3.4 and it fails for
>>>> external tables. Linux it is working fine. Let me know if I miss
>>>> anything.
>>>>
>>>> Please check the below logs.
>>>>
>>>> python runtests.py --pkg browser --exclude feature_tests
>>>>
>>>> 
>>>>
>>>> ==
>>>> ERROR: runTest
>>>> (pgadmin.browser.server_groups.servers.databases.schemas.tables.tests.test_template_create.TestTemplateCreate)
>>>> When rendering GreenPlum 5.3 template, when no distribution is present,
>>>> when no primary key is present, it returns "DISTRIBUTED RANDOMLY"
>>>> --
>>>> Traceback (most recent call last):
>>>>   File
>>>> "C:\Projects\pgadmin4\web\pgadmin\browser\server_groups\servers\databases\schemas\tables\tests\test_template_create.py",
>>>> line 99, in runTest
>>>> self.template_path, **self.input_parameters)
>>>>   File
>>>> "C:\Projects\venv_pgadmin4_py_3_4\lib\site-packages\flask\templating.py",
>>>> line 133, in render_template
>>>> return
>>>> _render(ctx.app.jinja_env.get_or_select_template(template_name_or_list),
>>>>   File
>>>> "C:\Projects\venv_pgadmin4_py_3_4\lib\site-packages\jinja2\environment.py",
>>>> line 830, in get_or_select_template
>>>> return self.get_template(template_name_or_list, parent, globals)
>>>>   File
>>>> "C:\Projects\venv_pgadmin4_py_3_4\lib\site-packages\jinja2\environment.py",
>>>> line 791, in get_template
>>>> return self._load_template(name, self.make_globals(globals))
>>>>   File
>>>> "C:\Projects\venv_pgadmin4_py_3_4\lib\site-packages\jinja2\environment.py",
>>>> line 765, in _load_template
>>>> template = self.loader.load(self, name, globals)
>>>>   File
>>>> "C:\Projects\venv_pgadmin4_py_3_4\lib\site-packages\jinja2\loaders.py",
>>>> line 113, in load
>>>> source, filename, uptodate = self.get_source(environment, name)
>>>>   File
>>>> "C:\Projects\venv_pgadmin4_py_3_4\lib\site-packages\flask\templating.py",
>>>> line 57, in get_source
>>>> return self._get_source_fast(environment, template)
>>>>   File
>>>> "C:\Projects\venv_pgadmin4_py_3_4\lib\site-packages\flask\templating.py",
>>>> line 85, in _get_source_fast
>>>> raise TemplateNotFound(template)
>>>> jinja2.exceptions.TemplateNotFound: table\sql\gpdb_5.0_plus\create.sql
>>>>
>>>> ==
>>>> ERROR: runTest
>>>> (pgadmin.browser.server_groups.servers.databases.schemas.tables.tests.test_template_create.TestTemplateCreate)
>>>> When render

Re: [pgadmin4][patch] Unit test fail on GreenPlum (#3190)

[Joao De Almeida Pereira]

Hello,
Can you point out an example?
Thanks

On Sat, Mar 10, 2018, 3:53 PM Dave Page <dp...@pgadmin.org> wrote:

> Hi
>
> On Friday, March 9, 2018, Joao De Almeida Pereira <
> jdealmeidapere...@pivotal.io> wrote:
>
>> Hello Hackers,
>>
>> Attached you can find the patch that skip some tests and correct issues
>> on SQL that are failing when trying to connect to a GreenPlum database.
>>
>> We did this by adding a attribute to to test_json called "db_type" that
>> will carry the type of database we are running tests against.
>>
>
> Any reason we can't do that dynamically as we do with the EPAS-specific
> tests?
>
>
>>
>> When we run tests against a GreenPlum instance the configuration would
>> look like this:
>>
>> {
>>   "name": "GreenPlum",
>>   "comment": "GreenPlum DB",
>>   "db_username": "gp",
>>   "host": "localhost",
>>   "db_password": "",
>>   "db_port": 5433,
>>   "maintenance_db": "postgres",
>>   "sslmode": "prefer",
>>   "tablespace_path": "",
>>   "enabled": true,
>>   "db_type": "gpdb"
>> }
>>
>>
>>
>>
>> Thanks
>> Joao
>>
>
>
> --
> Dave Page
> Blog: http://pgsnake.blogspot.com
> Twitter: @pgsnake
>
> EnterpriseDB UK: http://www.enterprisedb.com
> The Enterprise PostgreSQL Company
>
>

[pgadmin4][patch] Unit test fail on GreenPlum (#3190)

[Joao De Almeida Pereira]

Hello Hackers,

Attached you can find the patch that skip some tests and correct issues on
SQL that are failing when trying to connect to a GreenPlum database.

We did this by adding a attribute to to test_json called "db_type" that
will carry the type of database we are running tests against.

When we run tests against a GreenPlum instance the configuration would look
like this:

{
  "name": "GreenPlum",
  "comment": "GreenPlum DB",
  "db_username": "gp",
  "host": "localhost",
  "db_password": "",
  "db_port": 5433,
  "maintenance_db": "postgres",
  "sslmode": "prefer",
  "tablespace_path": "",
  "enabled": true,
  "db_type": "gpdb"
}




Thanks
Joao
diff --git a/web/pgadmin/browser/server_groups/servers/databases/casts/tests/test_cast_add.py b/web/pgadmin/browser/server_groups/servers/databases/casts/tests/test_cast_add.py
index 81f072f1..de8f9799 100644
--- a/web/pgadmin/browser/server_groups/servers/databases/casts/tests/test_cast_add.py
+++ b/web/pgadmin/browser/server_groups/servers/databases/casts/tests/test_cast_add.py
@@ -20,6 +20,7 @@ from . import utils as cast_utils
 
 
 class CastsAddTestCase(BaseTestGenerator):
+skip_on_database = ['gpdb']
 scenarios = [
 # Fetching default URL for cast node.
 ('Check Cast Node', dict(url='/browser/cast/obj/'))
@@ -27,6 +28,7 @@ class CastsAddTestCase(BaseTestGenerator):
 
 def runTest(self):
 """ This function will add cast under test database. """
+super(CastsAddTestCase, self).runTest()
 self.server_data = parent_node_dict["database"][-1]
 self.server_id = self.server_data["server_id"]
 self.db_id = self.server_data['db_id']
diff --git a/web/pgadmin/browser/server_groups/servers/databases/casts/tests/test_cast_delete.py b/web/pgadmin/browser/server_groups/servers/databases/casts/tests/test_cast_delete.py
index 46e2a013..b956fcbc 100644
--- a/web/pgadmin/browser/server_groups/servers/databases/casts/tests/test_cast_delete.py
+++ b/web/pgadmin/browser/server_groups/servers/databases/casts/tests/test_cast_delete.py
@@ -19,12 +19,14 @@ from . import utils as cast_utils
 
 class CastsDeleteTestCase(BaseTestGenerator):
 """ This class will delete the cast node added under database node. """
+skip_on_database = ['gpdb']
 scenarios = [
 # Fetching default URL for cast node.
 ('Check Cast Node', dict(url='/browser/cast/obj/'))
 ]
 
 def setUp(self):
+super(CastsDeleteTestCase, self).setUp()
 self.default_db = self.server["db"]
 self.database_info = parent_node_dict['database'][-1]
 self.db_name = self.database_info['db_name']
diff --git a/web/pgadmin/browser/server_groups/servers/databases/casts/tests/test_cast_get.py b/web/pgadmin/browser/server_groups/servers/databases/casts/tests/test_cast_get.py
index 329162eb..d67f55ae 100644
--- a/web/pgadmin/browser/server_groups/servers/databases/casts/tests/test_cast_get.py
+++ b/web/pgadmin/browser/server_groups/servers/databases/casts/tests/test_cast_get.py
@@ -19,6 +19,7 @@ from . import utils as cast_utils
 
 class CastsGetTestCase(BaseTestGenerator):
 """ This class will fetch the cast node added under database node. """
+skip_on_database = ['gpdb']
 scenarios = [
 # Fetching default URL for cast node.
 ('Check Cast Node', dict(url='/browser/cast/obj/'))
@@ -26,6 +27,7 @@ class CastsGetTestCase(BaseTestGenerator):
 
 def setUp(self):
 """ This function will create cast."""
+super(CastsGetTestCase, self).setUp()
 self.default_db = self.server["db"]
 self.database_info = parent_node_dict['database'][-1]
 self.db_name = self.database_info['db_name']
diff --git a/web/pgadmin/browser/server_groups/servers/databases/casts/tests/test_cast_put.py b/web/pgadmin/browser/server_groups/servers/databases/casts/tests/test_cast_put.py
index f3e43ae9..99485095 100644
--- a/web/pgadmin/browser/server_groups/servers/databases/casts/tests/test_cast_put.py
+++ b/web/pgadmin/browser/server_groups/servers/databases/casts/tests/test_cast_put.py
@@ -21,6 +21,7 @@ from . import utils as cast_utils
 
 class CastsPutTestCase(BaseTestGenerator):
 """ This class will fetch the cast node added under database node. """
+skip_on_database = ['gpdb']
 scenarios = [
 # Fetching default URL for cast node.
 ('Check Cast Node', dict(url='/browser/cast/obj/'))
@@ -28,6 +29,7 @@ class CastsPutTestCase(BaseTestGenerator):
 
 def setUp(self):
 """ This function will create cast."""
+super(CastsPutTestCase, self).setUp()
 self.default_db = self.server["db"]
 self.database_info = parent_node_dict['database'][-1]
 self.db_name = self.database_info['db_name']
diff --git a/web/pgadmin/browser/server_groups/servers/databases/extensions/tests/test_extension_add.py b/web/pgadmin/browser/server_groups/servers/databases/extensions/tests/test_extension_add.py
index 96bf6343..f4398d55 100644

Re: ACI Tree

[Joao De Almeida Pereira]

Hi Hackers,
Maybe this list might not have a big group of users of the application
itself, nevertheless it would be interesting to understand if this is a
specific problem of GreenPlum Use Case or not.
This issue is preventing a wider adoption of pgAdmin4 by the GreenPlum
users.

>From a preliminary investigation we found the following:
- When we try to retrieve 8k tables from the backend we get a payload of
3.3Mb with the following information:

   1. has_enable_triggers:"0"
   2. icon:"icon-table"
   3. id:"table/831144"
   4. inode:true
   5. is_partitioned:false
   6. label:"act_20141008"
   7. module:"pgadmin.node.table"
   8. rows_cnt:0
   9. tigger_count:"0"
   10. _id:831144
   11. _pid:24579
   12. _type:"table"

- This amount of information take around 12 seconds to be displayed
*- It is pretty hard to find something in set off 8k tables*

We started looking into possibilities to solve this issue, but we bumped
into the ACI Tree again and the way ACI Tree is so ingrained into our code.
In order to try to create a better experience  to these users these are the
steps that we believe need to be done:

1 - Refactor the code so that it doesn't depend on the Tree to run
2 - See if this allows us to have an increased performance.
3 - Instead of adding functionality to a Tree that doesn't look actively
supported, maybe we should look into other trees that are more actively
being worked on
4 - Eventually replace the tree with one that would allow us to have a
smaller footprint and have functionalities like search already embedded.

The last time we tried to take a look at ACI Tree we started by trying to
create a new Tree and see if we could plug it in the current code, and that
approach was not successful, so we believe that this new approach might
gain us the following:
 - Detachment from the tree creating an adapter layer that would eventually
would allow us to swap tree if that is the case
 - Try to simplify the information returned by the backend
 - Stop piggybacking on the alias of requirejs to have things done
 - Steer us into a direction where adding a feature to the tree is
something easy, testable and sustainable.


In a quick search we found 2 libraries that look interesting and actively
being developed:
https://www.npmjs.com/package/react-virtualized-tree
https://www.npmjs.com/package/react-infinite-tree

Here are some pros and cons on the libraries that we found:
react-virtualized-tree:
Pros:
- Actively developed
- Search capabilities
- Users react virtualized, which looks very interesting because it doesn't
dump everything into the dom at once
Cons:
- Single Committer

react-infinite-tree:
Pros:
- Actively developed
- Search capabilities
Cons:
- Single Committer

ACI Tree:
Pros:
- Already in the code
Cons:
- No longer maintained
- No website with documentation
- No search
- Heavy

Thanks
Joao


On Wed, Mar 7, 2018 at 12:19 PM Robert Eckhardt 
wrote:

> Hackers,
>
> We have multiple end users who have in excess of 10 thousand of tables in
> a single schema. Currently this causes pgAdmin to choke.
>
> The major issue we are seeing is that the ACI tree is unsupported and it
> seems to be the backbone of pgAdmin 4.
>
> Is anyone else having this issue?  Is there a solution better that (for
> some definition of better than) replacing the ACI Tree with something more
> performant?
>
> -- Rob
>

Re: pgAdmin 4 commit: Support EXPLAIN on Greenplum. Fixes #3097

[Joao De Almeida Pereira]

On Fri, Mar 9, 2018 at 11:04 AM Dave Page <dp...@pgadmin.org> wrote:

> Hi
>
> On Fri, Mar 9, 2018 at 3:54 PM, Joao De Almeida Pereira <
> jdealmeidapere...@pivotal.io> wrote:
>
>> Hello,
>> Definitely running single tests is something that would be great,
>> specially if you are TDDing something waiting 30-40 seconds to get feedback
>> is a little cumbersome when the test you are concerned with take less then
>> a second.
>>
>> In the process:
>> 1. Write a test
>> 2. Make the test pass
>> 3. Refactor
>>
>
> Sure, makes sense for development. As I spend 99% of my time reviewing and
> testing these days, I was just relaying my pain points :-)
>
>
>>
>> in between each step you run the test more then 1 time, and depending on
>> the refactoring you might need to run it several times. So imagine waiting
>> 30 seconds per run to get results. To run a subset of tests is a pain
>> because you need to be always changing the way you run the tests.
>>
>> I believe we could archive a better granularity and choosing what test to
>> run if we used a runner like pytest or nose to do it. What was the reason
>> behind handrolling a test runner script? I am asking this because in a
>> previous job I decided to handroll a unittest loader script and that was
>> something that I regretted every time I had to touch it, and eventually was
>> in the process of changing it to pytest.
>>
>
> Pure newbie-ism. I have no objections to changing to something else, if it
> reduces our tech debt.
>
>
>>
>> I looked into pytest to replace the current the current runtest, and the
>> major problem I found was the testscenarios integration(See Note 1). It can
>> be done but we would need to change all the test functions to receive the
>> scenario variables through arguments on the function. Also didn't dug much
>> into setting all the variables that we need there and all the environment.
>> The other issue that I do not like very much about pytest is the fact
>> that you loose the unittest assertion that is not so bad because there are
>> some neat libraries like: https://github.com/grappa-py/grappa,
>> https://github.com/ActivisionGameScience/assertpy,
>> https://github.com/dgilland/verify. Personally I really like the syntax
>> of Grapa, but the Veridfy one is pretty similar to Jasmine too.
>>
>> What are your thoughts?
>>
>
> Huh, I also really like the grappa syntax. It's nice and readable.
>
>
>>
>>
>>
>> Note 1: As an example of what our functions would have to look like you
>> can see:
>> https://github.com/OriMenashe/pytest-scenario/blob/master/tests/test_parametrize.py
>> As a example this class:
>>
>
> Without a diff, it's hard to be sure, but it looks like the only change
> was BaseTestGenerator to object on the first line?
>
See the function signature, that is the cumbersome issue

>
>
>> class ServersWithServiceIDAddTestCase(BaseTestGenerator):
>> """ This class will add the servers under default server group. """
>>
>> scenarios = [
>> # Fetch the default url for server object
>> (
>> 'Default Server Node url', dict(
>> url='/browser/server/obj/'
>> )
>> )
>> ]
>>
>> def setUp(self):
>> pass
>>
>> def runTest(self):
>> """ This function will add the server under default server group."""
>> url = "{0}{1}/".format(self.url, utils.SERVER_GROUP)
>> # Add service name in the config
>> self.server['service'] = "TestDB"
>> response = self.tester.post(
>> url,
>> data=json.dumps(self.server),
>> content_type='html/json'
>> )
>> self.assertEquals(response.status_code, 200)
>> response_data = json.loads(response.data.decode('utf-8'))
>> self.server_id = response_data['node']['_id']
>>
>> def tearDown(self):
>> """This function delete the server from SQLite """
>> utils.delete_server_with_api(self.tester, self.server_id)
>>
>> Would have to look changed to:
>>
>> class ServersWithServiceIDAddTestCase(object):
>> """ This class will add the servers under default server group. """
>>
>> scenarios = [
>> # Fetch the default url for server object
>>  

Re: pgAdmin 4 commit: Support EXPLAIN on Greenplum. Fixes #3097

[Joao De Almeida Pereira]

Hello,
Definitely running single tests is something that would be great, specially
if you are TDDing something waiting 30-40 seconds to get feedback is a
little cumbersome when the test you are concerned with take less then a
second.

In the process:
1. Write a test
2. Make the test pass
3. Refactor

in between each step you run the test more then 1 time, and depending on
the refactoring you might need to run it several times. So imagine waiting
30 seconds per run to get results. To run a subset of tests is a pain
because you need to be always changing the way you run the tests.

I believe we could archive a better granularity and choosing what test to
run if we used a runner like pytest or nose to do it. What was the reason
behind handrolling a test runner script? I am asking this because in a
previous job I decided to handroll a unittest loader script and that was
something that I regretted every time I had to touch it, and eventually was
in the process of changing it to pytest.

I looked into pytest to replace the current the current runtest, and the
major problem I found was the testscenarios integration(See Note 1). It can
be done but we would need to change all the test functions to receive the
scenario variables through arguments on the function. Also didn't dug much
into setting all the variables that we need there and all the environment.
The other issue that I do not like very much about pytest is the fact that
you loose the unittest assertion that is not so bad because there are some
neat libraries like: https://github.com/grappa-py/grappa,
https://github.com/ActivisionGameScience/assertpy,
https://github.com/dgilland/verify. Personally I really like the syntax of
Grapa, but the Veridfy one is pretty similar to Jasmine too.

What are your thoughts?



Note 1: As an example of what our functions would have to look like you can
see:
https://github.com/OriMenashe/pytest-scenario/blob/master/tests/test_parametrize.py
As a example this class:

class ServersWithServiceIDAddTestCase(BaseTestGenerator):
""" This class will add the servers under default server group. """

scenarios = [
# Fetch the default url for server object
(
'Default Server Node url', dict(
url='/browser/server/obj/'
)
)
]

def setUp(self):
pass

def runTest(self):
""" This function will add the server under default server group."""
url = "{0}{1}/".format(self.url, utils.SERVER_GROUP)
# Add service name in the config
self.server['service'] = "TestDB"
response = self.tester.post(
url,
data=json.dumps(self.server),
content_type='html/json'
)
self.assertEquals(response.status_code, 200)
response_data = json.loads(response.data.decode('utf-8'))
self.server_id = response_data['node']['_id']

def tearDown(self):
"""This function delete the server from SQLite """
utils.delete_server_with_api(self.tester, self.server_id)

Would have to look changed to:

class ServersWithServiceIDAddTestCase(object):
""" This class will add the servers under default server group. """

scenarios = [
# Fetch the default url for server object
(
'Default Server Node url', dict(
url='/browser/server/obj/'
)
)
]

def setUp(self):
pass

def runTest(self, url):
""" This function will add the server under default server group."""
url = "{0}{1}/".format(url, utils.SERVER_GROUP)
# Add service name in the config
self.server['service'] = "TestDB"
response = self.tester.post(
url,
data=json.dumps(self.server),
content_type='html/json'
)
self.assertEquals(response.status_code, 200)
response_data = json.loads(response.data.decode('utf-8'))
self.server_id = response_data['node']['_id']

def tearDown(self):
"""This function delete the server from SQLite """
utils.delete_server_with_api(self.tester, self.server_id)



Thanks
Joao

On Fri, Mar 9, 2018 at 8:31 AM Dave Page <dp...@pgadmin.org> wrote:

> On Thu, Mar 8, 2018 at 2:22 PM, Joao De Almeida Pereira <
> jdealmeidapere...@pivotal.io> wrote:
>
>> Hello Khushboo,
>> Completely forgot about this python "feature"...
>> Attached is the fix.
>>
>
> Thanks, applied.
>
>
>>
>> Just as a side question, does anyone else feel the pain of wanting to run
>> a single test using a IDE or the command line and not being able to?
>>
>
> Not really - the Python and JS tests are so quick

[pagdmin][patch] Update jasmine #3182

[Joao De Almeida Pereira]

Hi Hackers,

Attached you can find the patch that updates the version of jasmine to 3.0

Thanks
Joao
diff --git a/web/package.json b/web/package.json
index 2707b334..a4cb58d5 100644
--- a/web/package.json
+++ b/web/package.json
@@ -18,7 +18,7 @@
 "file-loader": "^0.11.2",
 "image-webpack-loader": "^3.3.1",
 "is-docker": "^1.1.0",
-"jasmine-core": "~2.9.0",
+"jasmine-core": "~3.1.0",
 "jasmine-enzyme": "~4.1.1",
 "karma": "~1.5.0",
 "karma-babel-preprocessor": "^6.0.1",
diff --git a/web/regression/javascript/gettext_spec.js b/web/regression/javascript/gettext_spec.js
index 3e027450..2ad9d87c 100644
--- a/web/regression/javascript/gettext_spec.js
+++ b/web/regression/javascript/gettext_spec.js
@@ -13,7 +13,8 @@ import translations from 'translations';
 describe('translate', function () {
   describe('when there is no translation', function () {
 it('returns the original string', function () {
-  expect(gettext('something to be translated')).toEqual('something to be translated');
+  expect(gettext('something else to be translated')).toEqual('something' +
+' else to be translated');
 });
 
 describe('when there are substitutions', function () {
diff --git a/web/yarn.lock b/web/yarn.lock
index 666a20e6..5e310d79 100644
--- a/web/yarn.lock
+++ b/web/yarn.lock
@@ -4267,9 +4267,9 @@ isurl@^1.0.0-alpha5:
 has-to-string-tag-x "^1.2.0"
 is-object "^1.0.1"
 
-jasmine-core@~2.9.0:
-  version "2.9.1"
-  resolved "https://registry.yarnpkg.com/jasmine-core/-/jasmine-core-2.9.1.tgz#b6bbc1d8e65250d56f5888461705ebeeeb88f22f;
+jasmine-core@~3.1.0:
+  version "3.1.0"
+  resolved "https://registry.yarnpkg.com/jasmine-core/-/jasmine-core-3.1.0.tgz#a4785e135d5df65024dfc9224953df585bd2766c;
 
 jasmine-enzyme@~4.1.1:
   version "4.1.1"

Re: [pgAdmin4][RM#3037] Allow user to disable Gravatar image

[Joao De Almeida Pereira]

Hello,

I just sent a video that talks exactly about this, in the journey to have a
more maintainable code and easier to navigate, we sometimes increase
complexity for a period of time in order to get to the state that we want
to be.

In particular in this example I tried the following:
1. Demonstrate that we do not need templates to get things done
2. How to get the information we need to the place where it is needed, like
moving the information of the server mode to the front end where decisions
should be made

Sometimes just because we can do a simple change, doesn't mean we should do
it because it might bit us in the future.


The PoC looks like a huge amount of code that is not even related to
Gravatar, when we could just change a template file or 2, I hear that. But
what I am talking about is a change of paradigm and Architecture where the
Frontend is responsible for displaying the application and the Backend is
only responsible for giving information that the Frontend Requires. Instead
of the current mixed architecture where some things are done in the
Frontend some things are done in the Backend a more predictable
architecture would benefit the application and specially the developers.

That is why I always insist in refactor code as much as possible to ensure
that the code is readable and helps people follow it. This doesn't mean
abstracting classes of functions to make to code more generic, it means as
an example to split 1 functions of 500 lines into 10 functions of 50 lines
each or even 50 functions of 10 lines each but that are readable and not a
"goat trails" of ifs and elses that are very hard to read and follow.


Thanks
Joao

On Tue, Mar 6, 2018 at 4:49 AM Dave Page <dp...@pgadmin.org> wrote:

> Hi
>
> On Tue, Mar 6, 2018 at 6:59 AM, Murtuza Zabuawala <
> murtuza.zabuaw...@enterprisedb.com> wrote:
>
>> Hi Joao,
>>
>> Your suggestion is good but in my own opinion it is overkill just to
>> replace code {{ username| gravatar }} as rest of the syntax is pure
>> HTML, This makes code much simpler and easy to understand.
>> Apart from that we will be rendering 'index.html' template anyways and I
>> don't see any extra overhead.
>>
>
> For this particular issue, I'm inclined to agree.
>
> I do however, like the idea of caching preferences (no-brainer really,
> assuming we ensure we update/invalidate the cache when needed), and using
> promises for accessing them.
>
>
>>
>> I may be wrong, Let's wait for the input from other community members.
>>
>> Regards,
>> Murtuza
>>
>> On Tue, Mar 6, 2018 at 1:17 AM, Joao De Almeida Pereira <
>> jdealmeidapere...@pivotal.io> wrote:
>>
>>> Hello,
>>> I understand that, but what do we win by using it? As you have noticed
>>> by now I am not very fond of using templates for everything, and I believe
>>> this is one of the things that we can skip templates.
>>> We might even shift this to a frontend concern and if we want there are
>>> node libraries to get gravatars.
>>>
>>> I was able to create a PoC as a sample of what I was talking about:
>>>  - Add gravatar library to package.json
>>>  - Created a Preferences cache. (js/preferences.js)
>>>- So this was the concept I was talking about in a previous email we
>>> talked about caching.
>>>   the getConfiguration and the getAllConfiguration are not great
>>> names, but they return a Promise that is consumed in the load_gravatar. The
>>> idea here is that we are caching the pgAdmin settings and when need need to
>>> consume them, we wait for it to be available.
>>>  - load_gravatar is using the pattern to retrieve the configuration from
>>> a possible cache and then apply the gravatar
>>>
>>>
>>> Things that need to be revisited in the PoC:
>>> - No tests, just some spiked code
>>> - Did not retrieve the username from the backend, we need to create an
>>> endpoint that give us this
>>> - The url is hardcoded to get the configuration
>>> - Maybe the Preferences is not the correct place to get server_mode
>>> configuration not sure, what do you think?
>>>
>>>
>>> Thanks
>>> Joao
>>>
>>> On Mon, Mar 5, 2018 at 11:21 AM Murtuza Zabuawala <
>>> murtuza.zabuaw...@enterprisedb.com> wrote:
>>>
>>>> Hi Joao,
>>>>
>>>> We are using Flask-Gravatar <https://pythonhosted.org/Flask-Gravatar/>
>>>> module for this and it is designed to work with template only.
>>>>
>>>> --
>>>> Regards,
>>>> Murtuza Zabuawala
>>>> Enter

Re: [pgAdmin4][RM#2989] To fix the issue in Table node

[Joao De Almeida Pereira]

Hi Murtuza,

The code change works, and I passed the patches through our pipeline and
everything is green.
Personally I would love this bug fixes to have refactored the function into
smaller chunk and made it more readable so that the next time someone need
to check out a problem in the same area it is easier. I understand that
without a good test coverage it is hard to have confidence while
refactoring, but we need to start somewhere.

@Hackers
Here is a video that I saw some time ago about refactoring existing code
and code complexity that is very interesting
https://www.youtube.com/watch?v=8bZh5LMaSmE
In this video Sandi Metz does the Gilded Rose Kata in a talk in RailsConf
2014, and with it tries to demonstrate that code can be refactored and with
that it make the code much more simpler. But the journey is not always
simple and the complexity will increase before it get simpler. It is a good
example of something that we can try with our code.


Thanks
Joao

On Tue, Mar 6, 2018 at 4:23 AM Murtuza Zabuawala <
murtuza.zabuaw...@enterprisedb.com> wrote:

> Hi,
>
> PFA patch to fix the issue in Table node where wrong sql was generated
> while altering column.
>
> --
> Regards,
> Murtuza Zabuawala
> EnterpriseDB: http://www.enterprisedb.com
> The Enterprise PostgreSQL Company
>
>

Re: [pgAdmin4][Patch]: RM #3135 - [Web based] Syntax error displayed when user try to insert data on table where primray key is in captial letters and table contains OIDS

[Joao De Almeida Pereira]

Hello Khushboo,

All tests pass on CI, and the code looks good. The only issue that I found
was the line:
 os.path.dirname(os.path.realpath(__file__)) + "/../templates"

that will not work on windows.
So I updated your patch with the change and it should be ready to merge.


Thanks
Joao

On Tue, Mar 6, 2018 at 7:33 AM Khushboo Vashi <
khushboo.va...@enterprisedb.com> wrote:

> Hi Joao,
>
> Thanks for reviewing.
>
> On Thu, Mar 1, 2018 at 8:06 PM, Joao De Almeida Pereira <
> jdealmeidapere...@pivotal.io> wrote:
>
>> Hello Kushboo,
>>
>> Can we add some Unit test to ensure this does not happen again?
>>
>> Please find the attached patch with the unit tests.
>
>> Thanks
>> Joao
>>
>> Thanks,
> Khushboo
>
>> On Thu, Mar 1, 2018 at 2:28 AM Khushboo Vashi <
>> khushboo.va...@enterprisedb.com> wrote:
>>
>>> Hi,
>>>
>>> Please find the attached patch to fix RM # 3135 - [Web based] Syntax
>>> error displayed when user try to insert data on table where primray key is
>>> in captial letters and table contains OIDS
>>>
>>> Thanks,
>>> Khushboo
>>>
>>
diff --git a/web/pgadmin/tools/sqleditor/templates/sqleditor/sql/default/insert.sql b/web/pgadmin/tools/sqleditor/templates/sqleditor/sql/default/insert.sql
index a4ed8729..7e3f8a07 100644
--- a/web/pgadmin/tools/sqleditor/templates/sqleditor/sql/default/insert.sql
+++ b/web/pgadmin/tools/sqleditor/templates/sqleditor/sql/default/insert.sql
@@ -6,5 +6,5 @@ INSERT INTO {{ conn|qtIdent(nsp_name, object_name) }} (
 {% for col in data_to_be_saved %}
 {% if not loop.first %}, {% endif %}%({{ col }})s::{{ data_type[col] }}{% endfor %}
 )
-{% if pk_names %} returning {{pk_names}}{% endif %}
+{% if pk_names and not has_oids %} returning {{pk_names}}{% endif %}
 {% if has_oids %} returning oid{% endif %};
diff --git a/web/pgadmin/tools/sqleditor/tests/test_view_data_templates.py b/web/pgadmin/tools/sqleditor/tests/test_view_data_templates.py
new file mode 100644
index ..5d02af2b
--- /dev/null
+++ b/web/pgadmin/tools/sqleditor/tests/test_view_data_templates.py
@@ -0,0 +1,217 @@
+##
+#
+# pgAdmin 4 - PostgreSQL Tools
+#
+# Copyright (C) 2013 - 2018, The pgAdmin Development Team
+# This software is released under the PostgreSQL Licence
+#
+##
+
+import os
+import re
+
+from flask import Flask, render_template
+from jinja2 import FileSystemLoader
+
+from pgadmin import VersionedTemplateLoader
+from pgadmin.utils.route import BaseTestGenerator
+from pgadmin.utils.driver import get_driver
+from config import PG_DEFAULT_DRIVER
+try:
+from collections import OrderedDict
+except ImportError:
+from ordereddict import OrderedDict
+
+
+class TestViewDataTemplates(BaseTestGenerator):
+"""
+This class validates the template query for
+inserting and selecting table data.
+"""
+data_to_be_saved = OrderedDict()
+data_to_be_saved['id'] = '1'
+data_to_be_saved['text'] = 'just test'
+scenarios = [
+(
+'When inserting and selecting table data with only PK',
+dict(
+insert_template_path=os.path.join('sqleditor',
+  'sql',
+  'default',
+  'insert.sql'),
+insert_parameters=dict(
+data_to_be_saved=data_to_be_saved,
+primary_keys=None,
+object_name='test_table',
+nsp_name='test_schema',
+data_type={'text': 'text', 'id': 'integer'},
+pk_names='id',
+has_oids=False
+),
+insert_expected_return_value='INSERT INTO'
+ ' test_schema.test_table'
+ ' (id, text) VALUES'
+ ' (%(id)s::integer, '
+ '%(text)s::text)'
+ ' returning id;',
+select_template_path=os.path.join('sqleditor',
+  'sql',
+  'default',
+  'select.sql'),
+select_parameters=dict(
+object_name='test_table',
+nsp_name='test_schema',
+primary_keys=OrderedDict([('id', 'int4')]),
+has_oids=False
+),
+select_expected_return_value='SELEC

Re: pgAdmin 4 commit: Ensure we pick up the messages from the current query

[Joao De Almeida Pereira]

Hello Khushboo,
Thanks for the changes here. Now everything looks good, and the tests all
pass.

Thanks
Joao

On Tue, Mar 6, 2018 at 5:09 AM Khushboo Vashi <
khushboo.va...@enterprisedb.com> wrote:

> On Mon, Mar 5, 2018 at 8:42 PM, Joao De Almeida Pereira <
> jdealmeidapere...@pivotal.io> wrote:
>
>> Hello Khushboo,
>>
> Looks like we are almost doen, just missing one PEP-8 issue:
>> → pycodestyle --config=.pycodestyle pgadmin/tools
>> pgadmin/tools/sqleditor/tests/test_poll_query_tool.py:46: [E126]
>> continuation line over-indented for hanging indent
>> 1   E126 continuation line over-indented for hanging indent
>> 1
>>
>>
>> Thanks Joao.
> Please find the attached updated patch.
>
>> The tests run successfully in our CI pipeline.
>>
>> Thanks
>> Joao
>>
>> Thanks,
> Khushboo
>
>> On Sun, Mar 4, 2018 at 11:51 PM Khushboo Vashi <
>> khushboo.va...@enterprisedb.com> wrote:
>>
>>> On Fri, Mar 2, 2018 at 6:55 PM, Dave Page <dp...@pgadmin.org> wrote:
>>>
>>>> Could you rebase this please? It no longer applies.
>>>>
>>>> Please find the attached updated patch.
>>>
>>>> Thanks.
>>>>
>>>> On Thu, Mar 1, 2018 at 5:56 AM, Khushboo Vashi <
>>>> khushboo.va...@enterprisedb.com> wrote:
>>>>
>>>>> Hi Joao,
>>>>>
>>>>> Thanks for reviewing.
>>>>>
>>>>> On Wed, Feb 28, 2018 at 8:55 PM, Joao De Almeida Pereira <
>>>>> jdealmeidapere...@pivotal.io> wrote:
>>>>>
>>>>>> Hello Khushboo,
>>>>>> After reviewing the patch I have the gut feeling that we do not have
>>>>>> enough test coverage on this issue, specially due to the intricate while
>>>>>> loop and conditions around the polling.
>>>>>> I think that this deserve Unit tests around it, When I say Unit Test
>>>>>> I am not talking about executing queries against the database, but do 
>>>>>> some
>>>>>> stubbing of the database so that we can control the flow that we want.
>>>>>>
>>>>> You are right. It needs more unit testing. I have checked below
>>>>> scenarios:
>>>>> 1. Returns 2 notices with data output
>>>>> 2. Returns 1000 notices with data output
>>>>> 3. No notices with data output
>>>>>
>>>>> By running above, I have checked, each time returned notices are
>>>>> accurate, no old notices are getting appended, it does not affect with the
>>>>> amount of messages (few, none or more).  Also, with the updated patch, I
>>>>> have made sure that all these queries run with the single transaction id
>>>>> (same connection).
>>>>>
>>>>> So, please let me know if you think I can add more things to this.
>>>>>
>>>>>>
>>>>>>
>>>>> It is a temptation to try to always do a Feature Test to test what we
>>>>>> want because it is "easier" to write and ultimately it is what users see,
>>>>>> but while 1 Feature Test runs we can run 200 Unit Tests that give us much
>>>>>> more confidence that the code is doing what we expect it to do.
>>>>>>
>>>>>> Right, so added regression tests instead of feature tests.
>>>>>
>>>>> This being said, I run the tests on the CI Pipeline and all tests
>>>>>> pass. Running pycodestyle fails due to some line sizes on the
>>>>>> psycopg2/__init__py. I believe that it is not what you changed, but since
>>>>>> you were changing the file it can be fixed it is just:
>>>>>>
>>>>>> pgadmin/utils/driver/psycopg2/__init__.py:1276: [E501] line too long
>>>>>> (81 > 79 characters)
>>>>>> pgadmin/utils/driver/psycopg2/__init__.py:1277: [E501] line too long
>>>>>> (91 > 79 characters)
>>>>>> pgadmin/utils/driver/psycopg2/__init__.py:1282: [E501] line too long
>>>>>> (81 > 79 characters)
>>>>>> pgadmin/utils/driver/psycopg2/__init__.py:1283: [E501] line too long
>>>>>> (91 > 79 characters)
>>>>>> 4   E501 line too long (81 > 79 characters)
>>>>>>
>>>>>> Fixed. Thanks for pointing out.
>>>>>
>>>>>>
>>

Re: pgAdmin 4 commit: Support for external tables in GPDB. Fixes #3168

[Joao De Almeida Pereira]

Hello Neel,

You can find attached the corrections of the path's needed for windows. The
fix should correct TestExternalTablesView and TestTemplateCreate but for
the ChangePasswordTestCase I need more information to help you out. We need
to understand what is the response that the endpoint /user_management/user
is returning.

Thanks
Joao

On Tue, Mar 6, 2018 at 2:29 AM Neel Patel 
wrote:

> Hi Joao,
>
> I ran the testsuite in windows 10 with Python 3.4 and it fails for
> external tables. Linux it is working fine. Let me know if I miss anything.
>
> Please check the below logs.
>
> python runtests.py --pkg browser --exclude feature_tests
>
> 
>
> ==
> ERROR: runTest
> (pgadmin.browser.server_groups.servers.databases.schemas.tables.tests.test_template_create.TestTemplateCreate)
> When rendering GreenPlum 5.3 template, when no distribution is present,
> when no primary key is present, it returns "DISTRIBUTED RANDOMLY"
> --
> Traceback (most recent call last):
>   File
> "C:\Projects\pgadmin4\web\pgadmin\browser\server_groups\servers\databases\schemas\tables\tests\test_template_create.py",
> line 99, in runTest
> self.template_path, **self.input_parameters)
>   File
> "C:\Projects\venv_pgadmin4_py_3_4\lib\site-packages\flask\templating.py",
> line 133, in render_template
> return
> _render(ctx.app.jinja_env.get_or_select_template(template_name_or_list),
>   File
> "C:\Projects\venv_pgadmin4_py_3_4\lib\site-packages\jinja2\environment.py",
> line 830, in get_or_select_template
> return self.get_template(template_name_or_list, parent, globals)
>   File
> "C:\Projects\venv_pgadmin4_py_3_4\lib\site-packages\jinja2\environment.py",
> line 791, in get_template
> return self._load_template(name, self.make_globals(globals))
>   File
> "C:\Projects\venv_pgadmin4_py_3_4\lib\site-packages\jinja2\environment.py",
> line 765, in _load_template
> template = self.loader.load(self, name, globals)
>   File
> "C:\Projects\venv_pgadmin4_py_3_4\lib\site-packages\jinja2\loaders.py",
> line 113, in load
> source, filename, uptodate = self.get_source(environment, name)
>   File
> "C:\Projects\venv_pgadmin4_py_3_4\lib\site-packages\flask\templating.py",
> line 57, in get_source
> return self._get_source_fast(environment, template)
>   File
> "C:\Projects\venv_pgadmin4_py_3_4\lib\site-packages\flask\templating.py",
> line 85, in _get_source_fast
> raise TemplateNotFound(template)
> jinja2.exceptions.TemplateNotFound: table\sql\gpdb_5.0_plus\create.sql
>
> ==
> ERROR: runTest
> (pgadmin.browser.server_groups.servers.databases.schemas.tables.tests.test_template_create.TestTemplateCreate)
> When rendering GreenPlum 5.3 template, when no distribution is present,
> when primary key is present, it returns "DISTRIBUTED BY (attr_primary_key)"
> --
> Traceback (most recent call last):
>   File
> "C:\Projects\pgadmin4\web\pgadmin\browser\server_groups\servers\databases\schemas\tables\tests\test_template_create.py",
> line 99, in runTest
> self.template_path, **self.input_parameters)
>   File
> "C:\Projects\venv_pgadmin4_py_3_4\lib\site-packages\flask\templating.py",
> line 133, in render_template
> return
> _render(ctx.app.jinja_env.get_or_select_template(template_name_or_list),
>   File
> "C:\Projects\venv_pgadmin4_py_3_4\lib\site-packages\jinja2\environment.py",
> line 830, in get_or_select_template
> return self.get_template(template_name_or_list, parent, globals)
>   File
> "C:\Projects\venv_pgadmin4_py_3_4\lib\site-packages\jinja2\environment.py",
> line 791, in get_template
> return self._load_template(name, self.make_globals(globals))
>   File
> "C:\Projects\venv_pgadmin4_py_3_4\lib\site-packages\jinja2\environment.py",
> line 765, in _load_template
> template = self.loader.load(self, name, globals)
>   File
> "C:\Projects\venv_pgadmin4_py_3_4\lib\site-packages\jinja2\loaders.py",
> line 113, in load
> source, filename, uptodate = self.get_source(environment, name)
>   File
> "C:\Projects\venv_pgadmin4_py_3_4\lib\site-packages\flask\templating.py",
> line 57, in get_source
> return self._get_source_fast(environment, template)
>   File
> "C:\Projects\venv_pgadmin4_py_3_4\lib\site-packages\flask\templating.py",
> line 85, in _get_source_fast
> raise TemplateNotFound(template)
> jinja2.exceptions.TemplateNotFound: table\sql\gpdb_5.0_plus\create.sql
>
> ==
> ERROR: runTest
> (pgadmin.browser.server_groups.servers.databases.schemas.tables.tests.test_template_create.TestTemplateCreate)
> When rendering GreenPlum 5.3 template, when distribution is present, it
> returns "DISTRIBUTED BY (attr1, attr2, attr4)"
> 

Re: [pgAdmin4][RM#3037] Allow user to disable Gravatar image

[Joao De Almeida Pereira]

Hello,
I understand that, but what do we win by using it? As you have noticed by
now I am not very fond of using templates for everything, and I believe
this is one of the things that we can skip templates.
We might even shift this to a frontend concern and if we want there are
node libraries to get gravatars.

I was able to create a PoC as a sample of what I was talking about:
 - Add gravatar library to package.json
 - Created a Preferences cache. (js/preferences.js)
   - So this was the concept I was talking about in a previous email we
talked about caching.
  the getConfiguration and the getAllConfiguration are not great names,
but they return a Promise that is consumed in the load_gravatar. The idea
here is that we are caching the pgAdmin settings and when need need to
consume them, we wait for it to be available.
 - load_gravatar is using the pattern to retrieve the configuration from a
possible cache and then apply the gravatar


Things that need to be revisited in the PoC:
- No tests, just some spiked code
- Did not retrieve the username from the backend, we need to create an
endpoint that give us this
- The url is hardcoded to get the configuration
- Maybe the Preferences is not the correct place to get server_mode
configuration not sure, what do you think?


Thanks
Joao

On Mon, Mar 5, 2018 at 11:21 AM Murtuza Zabuawala <
murtuza.zabuaw...@enterprisedb.com> wrote:

> Hi Joao,
>
> We are using Flask-Gravatar <https://pythonhosted.org/Flask-Gravatar/>
> module for this and it is designed to work with template only.
>
> --
> Regards,
> Murtuza Zabuawala
> EnterpriseDB: http://www.enterprisedb.com
> The Enterprise PostgreSQL Company
>
>
> On Mon, Mar 5, 2018 at 8:57 PM, Joao De Almeida Pereira <
> jdealmeidapere...@pivotal.io> wrote:
>
>> Hi Murtuza,
>>
>> Why are we doing this using templates? Can this be done with a request to
>> the backend to get the image and then retrieve the Gravatar if it is
>> defined or return a static image if not?
>>
>> +
>> +{% if config.SERVER_MODE %}
>>  window.onload = function(e){
>>   setTimeout(function() {
>> -   var gravatarImg = '> height="18" alt="Gravatar image for {{ username }}"> {{ username }} > class="caret">';
>> +   var gravatarImg = {{ IMG.PREPARE_HTML()|safe }}
>> //$('#navbar-menu .navbar-right > li > a').html(gravatarImg);
>> var navbarRight =
>> document.getElementById("navbar-menu").getElementsByClassName("navbar-right")[0];
>> if (navbarRight) {
>>
>> what if we have
>>
>> var gravatarImg = '> height="18" alt="Gravatar image for {{ username }}"> {{ username }} > class="caret">';
>>
>> And then the endpoint
>> /user/{{username}}/gravatar
>> would be responsible for returning a Gravatar or not depending on the
>> configuration?
>>
>>
>> Thanks
>> Joao
>>
>> On Mon, Mar 5, 2018 at 3:13 AM Murtuza Zabuawala <
>> murtuza.zabuaw...@enterprisedb.com> wrote:
>>
>>> Hi,
>>>
>>> PFA patch to disable Gravatar image in Server mode.
>>>
>>> Requirments & Issues:
>>> - For security reasons.
>>> - For systems which do not have internet access.
>>> - Hangs pgAdmin4 while loading the page if connection has no internet
>>> access (as described in the ticket)
>>>
>>> --
>>> Regards,
>>> Murtuza Zabuawala
>>> EnterpriseDB: http://www.enterprisedb.com
>>> The Enterprise PostgreSQL Company
>>>
>>>
>
diff --git a/web/package.json b/web/package.json
index 2707b334..8d62e7b5 100644
--- a/web/package.json
+++ b/web/package.json
@@ -66,6 +66,7 @@
 "exports-loader": "~0.6.4",
 "flotr2": "^0.1.0",
 "font-awesome": "^4.7.0",
+"gravatar": "^1.6.0",
 "hard-source-webpack-plugin": "^0.4.9",
 "immutability-helper": "^2.2.0",
 "imports-loader": "^0.7.1",
diff --git a/web/pgadmin/browser/templates/browser/index.html b/web/pgadmin/browser/templates/browser/index.html
index 58ff43f8..0901f44a 100644
--- a/web/pgadmin/browser/templates/browser/index.html
+++ b/web/pgadmin/browser/templates/browser/index.html
@@ -5,6 +5,7 @@ try {
 require(
 ['sources/generated/app.bundle'],
 function() {
+pgAdmin.applyAvatar(pgAdmin);
 },
 function() {
 /* TODO:: Show proper error dialog */
@@ -66,17 +67,6 @@ require.onResourceLoad = function (context, map, depMaps) {
 }, 400)
   }
 };
-window.onload = function(e){
- setTimeout(function() {
-   var gravatarImg = ' {{ username }} ';
-   

Re: [pgAdmin4][RM#3037] Allow user to disable Gravatar image

[Joao De Almeida Pereira]

Hi Murtuza,

Why are we doing this using templates? Can this be done with a request to
the backend to get the image and then retrieve the Gravatar if it is
defined or return a static image if not?

+
+{% if config.SERVER_MODE %}
 window.onload = function(e){
  setTimeout(function() {
-   var gravatarImg = ' {{ username }} ';
+   var gravatarImg = {{ IMG.PREPARE_HTML()|safe }}
//$('#navbar-menu .navbar-right > li > a').html(gravatarImg);
var navbarRight =
document.getElementById("navbar-menu").getElementsByClassName("navbar-right")[0];
if (navbarRight) {

what if we have

var gravatarImg = ' {{ username }} ';

And then the endpoint
/user/{{username}}/gravatar
would be responsible for returning a Gravatar or not depending on the
configuration?


Thanks
Joao

On Mon, Mar 5, 2018 at 3:13 AM Murtuza Zabuawala <
murtuza.zabuaw...@enterprisedb.com> wrote:

> Hi,
>
> PFA patch to disable Gravatar image in Server mode.
>
> Requirments & Issues:
> - For security reasons.
> - For systems which do not have internet access.
> - Hangs pgAdmin4 while loading the page if connection has no internet
> access (as described in the ticket)
>
> --
> Regards,
> Murtuza Zabuawala
> EnterpriseDB: http://www.enterprisedb.com
> The Enterprise PostgreSQL Company
>
>

Re: pgAdmin 4 commit: Ensure we pick up the messages from the current query

[Joao De Almeida Pereira]

Hello Khushboo,
Looks like we are almost doen, just missing one PEP-8 issue:
→ pycodestyle --config=.pycodestyle pgadmin/tools
pgadmin/tools/sqleditor/tests/test_poll_query_tool.py:46: [E126]
continuation line over-indented for hanging indent
1   E126 continuation line over-indented for hanging indent
1


The tests run successfully in our CI pipeline.

Thanks
Joao

On Sun, Mar 4, 2018 at 11:51 PM Khushboo Vashi <
khushboo.va...@enterprisedb.com> wrote:

> On Fri, Mar 2, 2018 at 6:55 PM, Dave Page <dp...@pgadmin.org> wrote:
>
>> Could you rebase this please? It no longer applies.
>>
>> Please find the attached updated patch.
>
>> Thanks.
>>
>> On Thu, Mar 1, 2018 at 5:56 AM, Khushboo Vashi <
>> khushboo.va...@enterprisedb.com> wrote:
>>
>>> Hi Joao,
>>>
>>> Thanks for reviewing.
>>>
>>> On Wed, Feb 28, 2018 at 8:55 PM, Joao De Almeida Pereira <
>>> jdealmeidapere...@pivotal.io> wrote:
>>>
>>>> Hello Khushboo,
>>>> After reviewing the patch I have the gut feeling that we do not have
>>>> enough test coverage on this issue, specially due to the intricate while
>>>> loop and conditions around the polling.
>>>> I think that this deserve Unit tests around it, When I say Unit Test I
>>>> am not talking about executing queries against the database, but do some
>>>> stubbing of the database so that we can control the flow that we want.
>>>>
>>> You are right. It needs more unit testing. I have checked below
>>> scenarios:
>>> 1. Returns 2 notices with data output
>>> 2. Returns 1000 notices with data output
>>> 3. No notices with data output
>>>
>>> By running above, I have checked, each time returned notices are
>>> accurate, no old notices are getting appended, it does not affect with the
>>> amount of messages (few, none or more).  Also, with the updated patch, I
>>> have made sure that all these queries run with the single transaction id
>>> (same connection).
>>>
>>> So, please let me know if you think I can add more things to this.
>>>
>>>>
>>>>
>>> It is a temptation to try to always do a Feature Test to test what we
>>>> want because it is "easier" to write and ultimately it is what users see,
>>>> but while 1 Feature Test runs we can run 200 Unit Tests that give us much
>>>> more confidence that the code is doing what we expect it to do.
>>>>
>>>> Right, so added regression tests instead of feature tests.
>>>
>>> This being said, I run the tests on the CI Pipeline and all tests pass.
>>>> Running pycodestyle fails due to some line sizes on the
>>>> psycopg2/__init__py. I believe that it is not what you changed, but since
>>>> you were changing the file it can be fixed it is just:
>>>>
>>>> pgadmin/utils/driver/psycopg2/__init__.py:1276: [E501] line too long
>>>> (81 > 79 characters)
>>>> pgadmin/utils/driver/psycopg2/__init__.py:1277: [E501] line too long
>>>> (91 > 79 characters)
>>>> pgadmin/utils/driver/psycopg2/__init__.py:1282: [E501] line too long
>>>> (81 > 79 characters)
>>>> pgadmin/utils/driver/psycopg2/__init__.py:1283: [E501] line too long
>>>> (91 > 79 characters)
>>>> 4   E501 line too long (81 > 79 characters)
>>>>
>>>> Fixed. Thanks for pointing out.
>>>
>>>>
>>>> Thanks
>>>> Joao
>>>>
>>>>
>>>> On Wed, Feb 28, 2018 at 6:49 AM Khushboo Vashi <
>>>> khushboo.va...@enterprisedb.com> wrote:
>>>>
>>>>> On Mon, Feb 26, 2018 at 10:02 PM, Dave Page <dp...@pgadmin.org> wrote:
>>>>>
>>>>>> Argh, I ran some tests, but didn't spot any lost messages in the
>>>>>> tests I ran. I'll revert the patch.
>>>>>>
>>>>>> Khushboo;
>>>>>>
>>>>>> Please look at the following:
>>>>>>
>>>>>> - Fix the patch so it doesn't drop messages.
>>>>>>
>>>>> Fixed.
>>>>> By default, the notice attribute of the connection object of psycopg 2
>>>>> only stores 50 notices. Once it reaches to 50 it starts from 1 again.
>>>>> To fix this I have changed the notice attribute from list to deque to
>>>>> append more messages. Currently I have kept the maximum limit at a t

[pgadmin4][patch] GreenPlum function statistics through an exception

[Joao De Almeida Pereira]

Hi Hackers,
You can find attached the resolution for issue 3176.
When trying to retrieve the statistics from a function in a GreenPlum
database an error is displayed, To fix this for version 5.X we decided to
remove the ability to get statistics.

Thanks
Joao
diff --git a/web/pgadmin/browser/server_groups/servers/databases/schemas/functions/static/js/function.js b/web/pgadmin/browser/server_groups/servers/databases/schemas/functions/static/js/function.js
index 7622cd0b..6e405165 100644
--- a/web/pgadmin/browser/server_groups/servers/databases/schemas/functions/static/js/function.js
+++ b/web/pgadmin/browser/server_groups/servers/databases/schemas/functions/static/js/function.js
@@ -92,7 +92,9 @@ define('pgadmin.node.function', [
   collection_type: 'coll-function',
   hasSQL: true,
   hasDepends: true,
-  hasStatistics: true,
+  hasStatistics: (treeInformation) => {
+return treeInformation.server.server_type !== 'gpdb';
+  },
   hasScriptTypes: ['create', 'select'],
   parent_type: ['schema', 'catalog'],
   Init: function() {
diff --git a/web/pgadmin/misc/statistics/static/js/statistics.js b/web/pgadmin/misc/statistics/static/js/statistics.js
index 39ff7bb3..3ff88a0a 100644
--- a/web/pgadmin/misc/statistics/static/js/statistics.js
+++ b/web/pgadmin/misc/statistics/static/js/statistics.js
@@ -1,8 +1,10 @@
 define('misc.statistics', [
   'sources/gettext', 'underscore', 'underscore.string', 'jquery', 'backbone',
   'pgadmin.browser', 'pgadmin.backgrid', 'alertify', 'sources/size_prettify',
+  'sources/misc/statistics/statistics',
 ], function(
-  gettext, _, S, $, Backbone, pgBrowser, Backgrid, Alertify, sizePrettify
+  gettext, _, S, $, Backbone, pgBrowser, Backgrid, Alertify, sizePrettify,
+  statisticsHelper
 ) {
 
   if (pgBrowser.NodeStatistics)
@@ -208,7 +210,7 @@ define('misc.statistics', [
 // Cache the current IDs for next time
 $(panel[0]).data('node-prop', cache_flag);
 
-if (node.hasStatistics) {
+if (statisticsHelper.nodeHasStatistics(node, item)) {
   msg = '';
   var timer;
   // Set the url, fetch the data and update the collection
diff --git a/web/pgadmin/static/js/misc/statistics/statistics.js b/web/pgadmin/static/js/misc/statistics/statistics.js
new file mode 100644
index ..2aabc75b
--- /dev/null
+++ b/web/pgadmin/static/js/misc/statistics/statistics.js
@@ -0,0 +1,15 @@
+//
+//
+// pgAdmin 4 - PostgreSQL Tools
+//
+// Copyright (C) 2013 - 2018, The pgAdmin Development Team
+// This software is released under the PostgreSQL Licence
+//
+//
+export function nodeHasStatistics(node, item) {
+  if(typeof(node.hasStatistics) === 'function') {
+const treeHierarchy = node.getTreeNodeHierarchy(item);
+return node.hasStatistics(treeHierarchy);
+  }
+  return node.hasStatistics;
+}
diff --git a/web/regression/javascript/misc/statistics/statistics_spec.js b/web/regression/javascript/misc/statistics/statistics_spec.js
new file mode 100644
index ..87540d10
--- /dev/null
+++ b/web/regression/javascript/misc/statistics/statistics_spec.js
@@ -0,0 +1,35 @@
+//
+//
+// pgAdmin 4 - PostgreSQL Tools
+//
+// Copyright (C) 2013 - 2018, The pgAdmin Development Team
+// This software is released under the PostgreSQL Licence
+//
+//
+import {nodeHasStatistics} from "../../../../pgadmin/static/js/misc/statistics/statistics";
+
+describe('#nodeHasStatistics', () => {
+  describe('when node hasStatistics is not a function', () => {
+it('return the value of hasStatistics', () => {
+  const node = {
+hasStatistics: true,
+  };
+  expect(nodeHasStatistics(node, {})).toBe(true);
+});
+  });
+
+  describe('when node hasStatistics is a function', () => {
+describe('when the function returns true', () => {
+  it('returns true', () => {
+const node = {
+  hasStatistics: () => true,
+  getTreeNodeHierarchy: jasmine.createSpy(),
+};
+const item = {};
+
+expect(nodeHasStatistics(node, item)).toBe(true);
+expect(node.getTreeNodeHierarchy).toHaveBeenCalledWith(item);
+  });
+});
+  });
+});

[pgadmin4][patch] External Tables for GreenPlum #3168

[Joao De Almeida Pereira]

Hello Hackers,

Attached you can find the patch that remove the External Tables from the
Tables node and create a new node at the database level for GreenPlum.
 - Do not display External Tables in the Tables node
 - Create a new node under databases for External Tables
 - Generate DDL for external tables
 - Generate properties for external tables

All these changes can only be seen when accessing a GreenPlum database.

Thanks
Joao
diff --git a/web/pgadmin/browser/server_groups/servers/databases/external_tables/__init__.py b/web/pgadmin/browser/server_groups/servers/databases/external_tables/__init__.py
new file mode 100644
index ..137c6c44
--- /dev/null
+++ b/web/pgadmin/browser/server_groups/servers/databases/external_tables/__init__.py
@@ -0,0 +1,275 @@
+##
+#
+# pgAdmin 4 - PostgreSQL Tools
+#
+# Copyright (C) 2013 - 2018, The pgAdmin Development Team
+# This software is released under the PostgreSQL Licence
+#
+##
+
+"""Implements External Tables Node"""
+import os
+from functools import wraps
+from gettext import gettext
+
+from flask import render_template
+
+from config import PG_DEFAULT_DRIVER
+from pgadmin.browser.collection import CollectionNodeModule
+from pgadmin.browser.server_groups.servers import databases
+from pgadmin.browser.server_groups.servers.databases \
+.external_tables.mapping_utils import map_execution_location
+from pgadmin.browser.server_groups.servers.databases \
+.external_tables.properties import Properties, \
+PropertiesTableNotFoundException, PropertiesException
+from pgadmin.browser.server_groups.servers.databases \
+.external_tables.reverse_engineer_ddl import ReverseEngineerDDL
+from pgadmin.browser.utils import PGChildNodeView
+from pgadmin.utils.ajax import make_json_response, make_response, \
+internal_server_error
+from pgadmin.utils.compile_template_name import compile_template_path
+from pgadmin.utils.driver import get_driver
+
+
+class ExternalTablesModule(CollectionNodeModule):
+"""
+class ExternalTablesModule(CollectionNodeModule)
+
+A module class for External Tables node derived from
+CollectionNodeModule.
+
+Methods:
+---
+* __init__(*args, **kwargs)
+  - Method is used to initialize the External Tables module
+and it's base module.
+
+* get_nodes(gid, sid, did)
+  - Method is used to generate the browser collection node.
+
+* script_load()
+  - Load the module script for External Tables, when any of
+the database node is initialized.
+"""
+
+NODE_TYPE = 'external_table'
+COLLECTION_LABEL = gettext("External Tables")
+
+def __init__(self, *args, **kwargs):
+"""
+Method is used to initialize the External tables module and
+it's base module.
+
+Args:
+*args:
+**kwargs:
+"""
+
+super(ExternalTablesModule, self).__init__(*args, **kwargs)
+self.max_ver = 0
+
+def get_nodes(self, gid, sid, did):
+yield self.generate_browser_collection_node(did)
+
+@property
+def script_load(self):
+"""
+Load the module script for External tables,
+when any of the database node is initialized.
+
+Returns: node type of the database module.
+"""
+return databases.DatabaseModule.NODE_TYPE
+
+@property
+def module_use_template_javascript(self):
+"""
+Returns whether Jinja2 template is used for generating the javascript
+module.
+"""
+return False
+
+
+blueprint = ExternalTablesModule(__name__)
+
+
+class ExternalTablesView(PGChildNodeView):
+node_type = blueprint.node_type
+
+parent_ids = [
+{'type': 'int', 'id': 'server_group_id'},
+{'type': 'int', 'id': 'server_id'},
+{'type': 'int', 'id': 'database_id'}
+]
+
+ids = [
+{'type': 'int', 'id': 'external_table_id'}
+]
+
+operations = dict({
+'obj': [
+{'get': 'properties'}
+],
+'nodes': [{'get': 'node'}, {'get': 'nodes'}],
+'sql': [{'get': 'sql'}],
+'children': [{'get': 'children'}]
+})
+
+def check_precondition(function_wrapped):
+"""
+This function will behave as a decorator which will checks
+database connection before running view, it will also attaches
+manager,conn & template_path properties to self
+"""
+
+@wraps(function_wrapped)
+def wrap(*args, **kwargs):
+# Here args[0] will hold self & kwargs will hold gid,sid,did
+self = args[0]
+self.manager = get_driver(PG_DEFAULT_DRIVER).connection_manager(
+kwargs['server_id']
+)
+self.connection = self.manager.connection(
+did=kwargs['database_id']
+)
+

Re: [pgAdmin4][RM#3129] handle encoding issue in File manager

[Joao De Almeida Pereira]

Hello Joao,
The pipeline is green and I believe the change is good to be merged.

Thanks
Joao

On Thu, Mar 1, 2018 at 10:56 AM Murtuza Zabuawala <
murtuza.zabuaw...@enterprisedb.com> wrote:

> Thanks Joao for reviewing.
>
> Attaching updated patch fixing PEP8 issues.
>
> --
> Regards,
> Murtuza Zabuawala
> EnterpriseDB: http://www.enterprisedb.com
> The Enterprise PostgreSQL Company
>
>
> On Thu, Mar 1, 2018 at 8:56 PM, Joao De Almeida Pereira <
> jdealmeidapere...@pivotal.io> wrote:
>
>> Hello Murtuza,
>>
>> The code looks pretty good, love the fact that you extracted it so that
>> our file size stop growing and get more manageable. All tests pass on our
>> CI.
>>
>> The only issue I found was linting:
>>
>> pycodestyle --config=.pycodestyle pgadmin/tools/sqleditor/
>> pgadmin/tools/sqleditor/utils/query_tool_fs_utils.py:12: [E302] expected
>> 2 blank lines, found 1
>> pgadmin/tools/sqleditor/utils/query_tool_fs_utils.py:53: [W391] blank
>> line at end of file
>> pgadmin/tools/sqleditor/utils/tests/test_query_tool_fs_utils.py:27:
>> [E121] continuation line under-indented for hanging indent
>> pgadmin/tools/sqleditor/utils/tests/test_query_tool_fs_utils.py:29:
>> [E122] continuation line missing indentation or outdented
>> 1   E121 continuation line under-indented for hanging indent
>> 1   E122 continuation line missing indentation or outdented
>> 1   E302 expected 2 blank lines, found 1
>> 1   W391 blank line at end of file
>> 4
>>
>> When this is fixed I think we are good to go
>>
>> Thanks
>> Joao
>>
>> On Thu, Mar 1, 2018 at 3:01 AM Murtuza Zabuawala <
>> murtuza.zabuaw...@enterprisedb.com> wrote:
>>
>>> Hi,
>>>
>>> PFA patch to fix the issue where user was not able to open the file with
>>> non utf-8 encoding.
>>>
>>>
>>> --
>>> Regards,
>>> Murtuza Zabuawala
>>> EnterpriseDB: http://www.enterprisedb.com
>>> The Enterprise PostgreSQL Company
>>>
>>>
>

Re: Bug #3083 fix

[Joao De Almeida Pereira]

Hello Neethu,
We passed the patch through our CI pipeline and all tests pass.
The code looks good, but we are trying to decouple files as much  as we can
so that we do not end up with files with over 1000 lines, that are hard to
read and to maintain. Also we are trying to create Unit Tests to have more
coverage in our Javascript code.

Can you split the new implementation code into it's own file and create
some tests to ensure the behavior will not be broken in the future?iYou
have some examples
on: pgadmin/browser/server_groups/servers/databases/external_tables/*

Thanks
Joao

On Thu, Mar 1, 2018 at 10:37 AM Neethu Mariya Joy 
wrote:

> Hi,
> I am Neethu Mariya Joy, an undergraduate pursuing BE in Computer Science
> at BITS Pilani.
>
> I've attempted to fix https://redmine.postgresql.org/issues/3083. Since
> the textarea resize feature is the default HTML feature, I have not changed
> it. Instead, I've added draggable borders to the wrapper which expands the
> textarea inside it.
>
> I'm attaching my patch as bug3083.diff below as per the contribution
> guidelines.
>
> Hope this helps. Thank you for your consideration!
>
> Sincerely,
> Neethu Mariya Joy
> GitHub  | Linkedin
> 
>
>
>

Re: Bug #2309 fix

[Joao De Almeida Pereira]

Hello Neethu,

We run the patch though our test pipeline and all tests are green.
Everything looks good with this patch

Thanks
Joao
On Thu, Mar 1, 2018 at 10:37 AM Neethu Mariya Joy 
wrote:

> Hi,
> I am Neethu Mariya Joy, an undergraduate pursuing BE in Computer Science
> at BITS Pilani.
>
> I've attempted to fix https://redmine.postgresql.org/issues/2309.
> Codemirrors catches all the keyboard and mouse events when 'readOnly'
> option is set to 'noCursor' and does not allow copying.
> So, I've set 'readOnly' option to true. In order to hide the cursor, I've
> added a class 'hide-cursor-workaround' and applied css styles to hide the
> cursor.
>
> I'm attaching my patch as bug2309.diff below as per the contribution
> guidelines.
>
> Hope this helps. Thank you for your consideration!
>
> Sincerely,
> Neethu Mariya Joy
> GitHub  | Linkedin
> 
>
>
>

Re: [pgAdmin4][RM#3129] handle encoding issue in File manager

[Joao De Almeida Pereira]

Hello Murtuza,

The code looks pretty good, love the fact that you extracted it so that our
file size stop growing and get more manageable. All tests pass on our CI.

The only issue I found was linting:

pycodestyle --config=.pycodestyle pgadmin/tools/sqleditor/
pgadmin/tools/sqleditor/utils/query_tool_fs_utils.py:12: [E302] expected 2
blank lines, found 1
pgadmin/tools/sqleditor/utils/query_tool_fs_utils.py:53: [W391] blank line
at end of file
pgadmin/tools/sqleditor/utils/tests/test_query_tool_fs_utils.py:27: [E121]
continuation line under-indented for hanging indent
pgadmin/tools/sqleditor/utils/tests/test_query_tool_fs_utils.py:29: [E122]
continuation line missing indentation or outdented
1   E121 continuation line under-indented for hanging indent
1   E122 continuation line missing indentation or outdented
1   E302 expected 2 blank lines, found 1
1   W391 blank line at end of file
4

When this is fixed I think we are good to go

Thanks
Joao

On Thu, Mar 1, 2018 at 3:01 AM Murtuza Zabuawala <
murtuza.zabuaw...@enterprisedb.com> wrote:

> Hi,
>
> PFA patch to fix the issue where user was not able to open the file with
> non utf-8 encoding.
>
>
> --
> Regards,
> Murtuza Zabuawala
> EnterpriseDB: http://www.enterprisedb.com
> The Enterprise PostgreSQL Company
>
>

Re: pgAdmin 4 commit: Ensure we pick up the messages from the current query

[Joao De Almeida Pereira]

Hello Khushboo,
The patch runs successfully in our CI with all tests passing.

I see the test that you created, and I do not understand why we need to
create tests that do HTTP requests in order to check something that is
executed against the database. What I was talking about in my previous
email was having tests that tested the function by itself.


(Copied from: https://jfiaffe.files.wordpress.com/2014/09/tests-pyramid.png)

This is the Testing Pyramid, there are a bunch of different drawings of it
and ways to explain it, but in broad stokes what is means is that we should
have the majority of tests around a Unit (that are some disagreements in
the community of what a Unit is) and a very small amount of Manual testing.
What Unit usually means is piece of code, it can be a function, it can be a
class or it can even be a module, but is something self contained. In
pgAdmin's case the majority of our tests go around the Integration Layer
because we are using HTTP requests in order to executing queries in the
database, so basically we are doing tests end to end in the backend, and
the cost time.

I do not want to held this patch back because of this, and I say this
because I have minimal confidence with the tests that you created, that
they would catch the majority of the problems, and hope that the majority
of the code is exercised by it.

Nevertheless I would like to challenge all the Hackers to think about
testing in a different way. The tests in our code are used to give us
confidence that the work we did is working as expected, this also makes it
much easier to refactor out bad patterns or very complicated ones into
something simple. A signal that our code is more complicated then it should
is when we need to test some behavior and we end up with a Stubbing Hell or
we need to test it End 2 End because it is to hard to isolate the part we
want to test. In the other hand we should not test all functions and every
class, because we might be coupling our tests to much to the implementation
and that will have the contrary effect, and we will not be able to refactor
and simply our code.
Like everything in life there need to be a balance.

Thanks
Joao

On Thu, Mar 1, 2018 at 12:56 AM Khushboo Vashi <
khushboo.va...@enterprisedb.com> wrote:

> Hi Joao,
>
> Thanks for reviewing.
>
> On Wed, Feb 28, 2018 at 8:55 PM, Joao De Almeida Pereira <
> jdealmeidapere...@pivotal.io> wrote:
>
>> Hello Khushboo,
>> After reviewing the patch I have the gut feeling that we do not have
>> enough test coverage on this issue, specially due to the intricate while
>> loop and conditions around the polling.
>> I think that this deserve Unit tests around it, When I say Unit Test I am
>> not talking about executing queries against the database, but do some
>> stubbing of the database so that we can control the flow that we want.
>>
> You are right. It needs more unit testing. I have checked below scenarios:
> 1. Returns 2 notices with data output
> 2. Returns 1000 notices with data output
> 3. No notices with data output
>
> By running above, I have checked, each time returned notices are accurate,
> no old notices are getting appended, it does not affect with the amount of
> messages (few, none or more).  Also, with the updated patch, I have made
> sure that all these queries run with the single transaction id (same
> connection).
>
> So, please let me know if you think I can add more things to this.
>
>>
>>
> It is a temptation to try to always do a Feature Test to test what we want
>> because it is "easier" to write and ultimately it is what users see, but
>> while 1 Feature Test runs we can run 200 Unit Tests that give us much more
>> confidence that the code is doing what we expect it to do.
>>
>> Right, so added regression tests instead of feature tests.
>
> This being said, I run the tests on the CI Pipeline and all tests pass.
>> Running pycodestyle fails due to some line sizes on the
>> psycopg2/__init__py. I believe that it is not what you changed, but since
>> you were changing the file it can be fixed it is just:
>>
>> pgadmin/utils/driver/psycopg2/__init__.py:1276: [E501] line too long (81
>> > 79 characters)
>> pgadmin/utils/driver/psycopg2/__init__.py:1277: [E501] line too long (91
>> > 79 characters)
>> pgadmin/utils/driver/psycopg2/__init__.py:1282: [E501] line too long (81
>> > 79 characters)
>> pgadmin/utils/driver/psycopg2/__init__.py:1283: [E501] line too long (91
>> > 79 characters)
>> 4   E501 line too long (81 > 79 characters)
>>
>> Fixed. Thanks for pointing out.
>
>>
>> Thanks
>> Joao
>>
>>
>> On Wed, Feb 28, 2018 at 6:49 AM Khushboo Vashi <
>> khushboo.va...@enterprisedb.

Re: [pgAdmin4][Patch]: PEP-8 fixes in the foreign data wrapper module

[Joao De Almeida Pereira]

Hello Khushboo,
I applied this patch and here is the result:

 2018-03-01 09:41:00 ⌚ |ruby-2.4.1| pgadmin-dev in ~/workspace/pgadmin4/web
± |pep-8-fdw {2} U:13 ✗| → git st
On branch pep-8-fdw
Changes not staged for commit:
  (use "git add ..." to update what will be committed)
  (use "git checkout -- ..." to discard changes in working directory)

modified:
 
pgadmin/browser/server_groups/servers/databases/foreign_data_wrappers/__init__.py
modified:
 
pgadmin/browser/server_groups/servers/databases/foreign_data_wrappers/foreign_servers/__init__.py
modified:
 
pgadmin/browser/server_groups/servers/databases/foreign_data_wrappers/foreign_servers/tests/test_foreign_servers_add.py
modified:
 
pgadmin/browser/server_groups/servers/databases/foreign_data_wrappers/foreign_servers/tests/test_foreign_servers_delete.py
modified:
 
pgadmin/browser/server_groups/servers/databases/foreign_data_wrappers/foreign_servers/tests/test_foreign_servers_get.py
modified:
 
pgadmin/browser/server_groups/servers/databases/foreign_data_wrappers/foreign_servers/tests/test_foreign_servers_put.py
modified:
 
pgadmin/browser/server_groups/servers/databases/foreign_data_wrappers/foreign_servers/tests/utils.py
modified:
 
pgadmin/browser/server_groups/servers/databases/foreign_data_wrappers/foreign_servers/user_mapping/__init__.py
modified:
 
pgadmin/browser/server_groups/servers/databases/foreign_data_wrappers/foreign_servers/user_mapping/tests/test_user_mapping_add.py
modified:
 
pgadmin/browser/server_groups/servers/databases/foreign_data_wrappers/foreign_servers/user_mapping/tests/test_user_mapping_delete.py
modified:
 
pgadmin/browser/server_groups/servers/databases/foreign_data_wrappers/foreign_servers/user_mapping/tests/test_user_mapping_get.py
modified:
 
pgadmin/browser/server_groups/servers/databases/foreign_data_wrappers/foreign_servers/user_mapping/tests/test_user_mapping_put.py
modified:
 
pgadmin/browser/server_groups/servers/databases/foreign_data_wrappers/foreign_servers/user_mapping/tests/utils.py

no changes added to commit (use "git add" and/or "git commit -a")

 2018-03-01 09:41:03 ⌚ |ruby-2.4.1| pgadmin-dev in ~/workspace/pgadmin4/web
± |pep-8-fdw {2} U:13 ✗| → pycodestyle --config=.pycodestyle
pgadmin/browser/server_groups/servers/databases/foreign_data_wrappers/
pgadmin/browser/server_groups/servers/databases/foreign_data_wrappers/__init__.py:644:
[E123] closing bracket does not match indentation of opening bracket's line
pgadmin/browser/server_groups/servers/databases/foreign_data_wrappers/__init__.py:651:
[E123] closing bracket does not match indentation of opening bracket's line
pgadmin/browser/server_groups/servers/databases/foreign_data_wrappers/foreign_servers/__init__.py:593:
[E123] closing bracket does not match indentation of opening bracket's line
pgadmin/browser/server_groups/servers/databases/foreign_data_wrappers/foreign_servers/__init__.py:662:
[E123] closing bracket does not match indentation of opening bracket's line
pgadmin/browser/server_groups/servers/databases/foreign_data_wrappers/foreign_servers/__init__.py:669:
[E123] closing bracket does not match indentation of opening bracket's line
pgadmin/browser/server_groups/servers/databases/foreign_data_wrappers/foreign_servers/user_mapping/__init__.py:622:
[E123] closing bracket does not match indentation of opening bracket's line
pgadmin/browser/server_groups/servers/databases/foreign_data_wrappers/foreign_servers/user_mapping/__init__.py:685:
[E123] closing bracket does not match indentation of opening bracket's line
pgadmin/browser/server_groups/servers/databases/foreign_data_wrappers/foreign_servers/user_mapping/__init__.py:692:
[E123] closing bracket does not match indentation of opening bracket's line
pgadmin/browser/server_groups/servers/databases/foreign_data_wrappers/foreign_servers/user_mapping/tests/test_user_mapping_add.py:72:
[E126] continuation line over-indented for hanging indent
pgadmin/browser/server_groups/servers/databases/foreign_data_wrappers/foreign_servers/user_mapping/tests/test_user_mapping_add.py:84:
[E121] continuation line under-indented for hanging indent
pgadmin/browser/server_groups/servers/databases/foreign_data_wrappers/foreign_servers/user_mapping/tests/test_user_mapping_get.py:74:
[E126] continuation line over-indented for hanging indent
pgadmin/browser/server_groups/servers/databases/foreign_data_wrappers/foreign_servers/tests/test_foreign_servers_put.py:69:
[E126] continuation line over-indented for hanging indent
pgadmin/browser/server_groups/servers/databases/foreign_data_wrappers/foreign_servers/tests/test_foreign_servers_put.py:71:
[E121] continuation line under-indented for hanging indent
pgadmin/browser/server_groups/servers/databases/foreign_data_wrappers/tests/test_fdw_delete.py:25:
[E126] continuation line over-indented for hanging indent
pgadmin/browser/server_groups/servers/databases/foreign_data_wrappers/tests/test_fdw_delete.py:28:
[E121] continuation line under-indented for hanging indent

Re: [pgAdmin4][Patch]: RM #3135 - [Web based] Syntax error displayed when user try to insert data on table where primray key is in captial letters and table contains OIDS

[Joao De Almeida Pereira]

Hello Kushboo,

Can we add some Unit test to ensure this does not happen again?

Thanks
Joao

On Thu, Mar 1, 2018 at 2:28 AM Khushboo Vashi <
khushboo.va...@enterprisedb.com> wrote:

> Hi,
>
> Please find the attached patch to fix RM # 3135 - [Web based] Syntax error
> displayed when user try to insert data on table where primray key is in
> captial letters and table contains OIDS
>
> Thanks,
> Khushboo
>

Re: [pgAdmin4][RM#3161] Fix PEP-8 issues

[Joao De Almeida Pereira]

Hello Murtuza,

I passed the patch through our CI and all test are green. Also run the
codestyle in all those folders and everything is clean.

575 issues to go

Thanks
Joao

On Wed, Feb 28, 2018 at 1:05 PM Murtuza Zabuawala <
murtuza.zabuaw...@enterprisedb.com> wrote:

> Hi,
>
> PFA patch to fix the PEP8 issues in Casts, Event triggers, Extensions and
> Languages module.
>
> pycodestyle --config=.pycodestyle
> pgadmin/browser/server_groups/servers/databases/languages/
> pycodestyle --config=.pycodestyle
> pgadmin/browser/server_groups/servers/databases/extensions/
> pycodestyle --config=.pycodestyle
> pgadmin/browser/server_groups/servers/databases/event_triggers/
> pycodestyle --config=.pycodestyle
> pgadmin/browser/server_groups/servers/databases/casts/
>
> --
> Regards,
> Murtuza Zabuawala
> EnterpriseDB: http://www.enterprisedb.com
> The Enterprise PostgreSQL Company
>
>

Re: Feature #3061

[Joao De Almeida Pereira]

Hello Neethu
Welcome to the pgadmin-hackers, nice to see you here.

In order to start this story, I believe that we need to talk with some
users to understand what does this story mean. At this point the issue is
to broad and I personally do not understand what customization means.

But feel free to start having ideas on what does customization mean, and we
can create some prototypes and talk with users to see what their reaction
is to them.

Thanks
Joao

On Wed, Feb 28, 2018 at 7:59 AM Neethu Mariya Joy 
wrote:

> Hi,
>
> I am Neethu Mariya Joy, an undergraduate sophomore pursuing BE(Hons) in
> Computer Engineering from BITS Pilani, India.
>
> I would like to work on the feature #3061, "Dashboard Customisation".
> Kindly provide suggestions for the same.
>
> Sincerely,
> Neethu Mariya Joy
> GitHub  | Linkedin
> 
>

Re: pgAdmin 4 commit: Ensure we pick up the messages from the current query

[Joao De Almeida Pereira]

Hello Khushboo,
After reviewing the patch I have the gut feeling that we do not have enough
test coverage on this issue, specially due to the intricate while loop and
conditions around the polling.
I think that this deserve Unit tests around it, When I say Unit Test I am
not talking about executing queries against the database, but do some
stubbing of the database so that we can control the flow that we want.

It is a temptation to try to always do a Feature Test to test what we want
because it is "easier" to write and ultimately it is what users see, but
while 1 Feature Test runs we can run 200 Unit Tests that give us much more
confidence that the code is doing what we expect it to do.

This being said, I run the tests on the CI Pipeline and all tests pass.
Running pycodestyle fails due to some line sizes on the
psycopg2/__init__py. I believe that it is not what you changed, but since
you were changing the file it can be fixed it is just:

pgadmin/utils/driver/psycopg2/__init__.py:1276: [E501] line too long (81 >
79 characters)
pgadmin/utils/driver/psycopg2/__init__.py:1277: [E501] line too long (91 >
79 characters)
pgadmin/utils/driver/psycopg2/__init__.py:1282: [E501] line too long (81 >
79 characters)
pgadmin/utils/driver/psycopg2/__init__.py:1283: [E501] line too long (91 >
79 characters)
4   E501 line too long (81 > 79 characters)


Thanks
Joao


On Wed, Feb 28, 2018 at 6:49 AM Khushboo Vashi <
khushboo.va...@enterprisedb.com> wrote:

> On Mon, Feb 26, 2018 at 10:02 PM, Dave Page  wrote:
>
>> Argh, I ran some tests, but didn't spot any lost messages in the tests I
>> ran. I'll revert the patch.
>>
>> Khushboo;
>>
>> Please look at the following:
>>
>> - Fix the patch so it doesn't drop messages.
>>
> Fixed.
> By default, the notice attribute of the connection object of psycopg 2
> only stores 50 notices. Once it reaches to 50 it starts from 1 again.
> To fix this I have changed the notice attribute from list to deque to
> append more messages. Currently I have kept the maximum limit at a time of
> the notice attribute is 10 (in a single poll).
>
>> - Add regression tests to make sure it doesn't break in the future. This
>> may require creating one or more functions the spew out a whole lot of
>> notices, and then running a couple of queries and checking the output.
>>
> Added. With this regression test, the current code is failing which has
> been taken care in this patch.
>
>> - Check the messages panel on the history tab. I just noticed it seems to
>> only be showing an even smaller subset of the messages.
>>
> Tested and no issues found.
>
>>
>>
> Thanks.
>>
>> On Mon, Feb 26, 2018 at 4:23 PM, Murtuza Zabuawala <
>> murtuza.zabuaw...@enterprisedb.com> wrote:
>>
>>> Sent bit early,
>>>
>>> You can run 'VACUUM FULL VERBOSE' in query tool and verify the
>>> populated messages (pgAdmin3 vs. pgAdmin4).
>>>
>>>
>>> On Mon, Feb 26, 2018 at 9:48 PM, Murtuza Zabuawala <
>>> murtuza.zabuaw...@enterprisedb.com> wrote:
>>>
 Hi Khushboo/Dave,

 With given commit, I'm again seeing the issue raised in
 https://redmine.postgresql.org/issues/1523 :(




 --
 Regards,
 Murtuza Zabuawala
 EnterpriseDB: http://www.enterprisedb.com
 The Enterprise PostgreSQL Company


 On Mon, Feb 26, 2018 at 7:49 PM, Dave Page  wrote:

> Ensure we pick up the messages from the current query and not a
> previous one. Fixes #3094
>
> Branch
> --
> master
>
> Details
> ---
>
> https://git.postgresql.org/gitweb?p=pgadmin4.git;a=commitdiff;h=08b3ccc01a4d57e8ea3657f8882a53dcd1b99386
> Author: Khushboo Vashi 
>
> Modified Files
> --
> web/pgadmin/utils/driver/abstract.py  |  1 +
> web/pgadmin/utils/driver/psycopg2/__init__.py | 64
> +--
> 2 files changed, 21 insertions(+), 44 deletions(-)
>
>

>>>
>>
>>
>> --
>> Dave Page
>> Blog: http://pgsnake.blogspot.com
>> Twitter: @pgsnake
>>
>> EnterpriseDB UK: http://www.enterprisedb.com
>> The Enterprise PostgreSQL Company
>>
>

Re: [pgAdmin4][RM#3073] Allow user to schedule without End date from UI

[Joao De Almeida Pereira]

Hello Murtuza,
I do not have the pgAgent installed so it was a little it hard to test this.
After looking into the code I think we are missing some testing coverage
around the pga_job Javascript part so that we can catch these problems.
Even some code extraction can be done around the pg_jobset change

Nevertheless I passed the patch through our CI and it is all green

Thanks
Joao

On Wed, Feb 28, 2018 at 5:36 AM Murtuza Zabuawala <
murtuza.zabuaw...@enterprisedb.com> wrote:

> Hi Dave,
>
> I have found the issue, it was in Backform control itself :)
> Issue: We were passing today's date value as minDate option in pgAgent
> schedule schema while rendering the control, So when server was sending
> previous datetime value it was not displaying it causing sync problem.
>
> --
> Regards,
> Murtuza Zabuawala
> EnterpriseDB: http://www.enterprisedb.com
> The Enterprise PostgreSQL Company
>
>
> On Tue, Feb 27, 2018 at 8:39 PM, Murtuza Zabuawala <
> murtuza.zabuaw...@enterprisedb.com> wrote:
>
>> ​Thanks Dave, I'll look into it.
>> ​
>>
>>
>> On Tue, Feb 27, 2018 at 8:37 PM, Dave Page  wrote:
>>
>>> Hi
>>>
>>> On Tue, Feb 27, 2018 at 12:03 PM, Murtuza Zabuawala <
>>> murtuza.zabuaw...@enterprisedb.com> wrote:
>>>
 I'm not able to re-produce the issue, Could you hard refresh and try
 again?

>>>
>>> I still see it.
>>>
>>>

 If possible could you please provide exact steps?

>>>
>>> Attached is a dump of my test pgagent schema.
>>>
>>> 1) Load the schema dump into the maintenance database
>>> 2) Connect pgAdmin
>>> 3) Browse to pgAgent Jobs
>>> 4) Right-click Properties, and select the Schedules tab
>>> 5) Open the subnode for sched1
>>>
>>>

 --
 Regards,
 Murtuza Zabuawala
 EnterpriseDB: http://www.enterprisedb.com
 The Enterprise PostgreSQL Company


 On Tue, Feb 27, 2018 at 5:30 PM, Dave Page  wrote:

> Sorry - here it is.
>
> On Tue, Feb 27, 2018 at 11:59 AM, Murtuza Zabuawala <
> murtuza.zabuaw...@enterprisedb.com> wrote:
>
>> Hi Dave,
>>
>> Could you please send screenshot?
>>
>> --
>> Regards,
>> Murtuza Zabuawala
>> EnterpriseDB: http://www.enterprisedb.com
>> The Enterprise PostgreSQL Company
>>
>>
>> On Tue, Feb 27, 2018 at 5:24 PM, Dave Page  wrote:
>>
>>> Hi
>>>
>>> Still not quite right - see the attached screenshot which is the
>>> result of simply viewing the properties of an existing job. Note that 
>>> the
>>> start time is shown in the grid but not the subnode control.
>>>
>>> Thanks.
>>>
>>> On Tue, Feb 27, 2018 at 8:26 AM, Murtuza Zabuawala <
>>> murtuza.zabuaw...@enterprisedb.com> wrote:
>>>
 Hi Dave,

 As validation related patch was committed with RM#3148 [ Sorry
 about that I forgot to checkout :) ]
 PFA patch to fix the issues you mentioned, I have also removed
 extra error message from sub node collection control and made it 
 optional
 via flag.



 --
 Regards,
 Murtuza Zabuawala
 EnterpriseDB: http://www.enterprisedb.com
 The Enterprise PostgreSQL Company


 On Mon, Feb 26, 2018 at 10:14 PM, Dave Page 
 wrote:

> Hi
>
> On Mon, Feb 26, 2018 at 2:46 PM, Murtuza Zabuawala <
> murtuza.zabuaw...@enterprisedb.com> wrote:
>
>> Hi,
>>
>> PFA patch to fix the issue where user was not able to create
>> pgAgent job from UI without entering End date in schedule section.
>>
>
>  Whilst this does resolve the validation issue, there are still a
> couple of other related problems, as can be seen in the attached
> screenshots:
>
> - The Start date/time in the subnode control doesn't seem to be
> properly synchronised with the value in the grid.
>
> - If you leave the End date/time blank (but maybe click into it
> first), the grid will show "Invalid date".
>
> It's possible there are other oddities as well - please check
> carefully for anything else.
>
> 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: 

Re: RM3079 fix for wrong sql datetime/time related datatypes

[Joao De Almeida Pereira]

Hello Harshal,

I tried the example you showed and it works. Also passed the patch through
our CI Pipeline and everything is good.

Thanks
Joao

On Wed, Feb 28, 2018 at 6:25 AM Harshal Dhumal <
harshal.dhu...@enterprisedb.com> wrote:

> Hi,
>
> On Tue, Feb 27, 2018 at 8:54 PM, Dave Page  wrote:
>
>> Hi
>>
>> On Tue, Feb 27, 2018 at 2:36 PM, Harshal Dhumal <
>> harshal.dhu...@enterprisedb.com> wrote:
>>
>>> Hi,
>>>
>>> Please find patch to fix wrong sql issue for time related type.
>>>
>>> Steps to reproduce:
>>>
>>> Alter any time/datetime array related data type, it generates sql with
>>> addition array bracket
>>>
>>> [image: Inline image 1]
>>>
>>
>> This seems to be missing the test case that you noted should be included
>> in the original bug report!
>>
>
> Please find updated patch with test cases.
>
>
> Thanks,
>
>>
>> --
>> Dave Page
>> Blog: http://pgsnake.blogspot.com
>> Twitter: @pgsnake
>>
>> EnterpriseDB UK: http://www.enterprisedb.com
>> The Enterprise PostgreSQL Company
>>
>

Re: [pgAdmin4][RM#3002] To fix the indentation issue in query tool editor

[Joao De Almeida Pereira]

Hello Murtuza,

The patch looks good, and solves the problem. Run the patch on our CI and
everything is green.

Thanks
Joao

On Tue, Feb 27, 2018 at 8:11 AM Murtuza Zabuawala <
murtuza.zabuaw...@enterprisedb.com> wrote:

> Hi,
>
> PFA patch to fix the issue where Tab key was not working as expected in
> query tool editor.
>
> Steps to reproduce:
> 1) Type any multiline sql/text.
> 2) select the complete sql by pressing Ctrl+A.
> 3) Press Tab key multiple times, you will see the problem in code
> indentation.
>
> Please review.
>
> --
> Regards,
> Murtuza Zabuawala
> EnterpriseDB: http://www.enterprisedb.com
> The Enterprise PostgreSQL Company
>
>