Re: [pgadmin-hackers][patch] Correct bundle test that polluted Python modules

[Joao Pedro De Almeida Pereira]

Hi Hackers,

Good catch Dave.

You can find attached the new diff with the correction of the problem above.

Thanks
Shruti & Joao

On Tue, Jun 13, 2017 at 10:35 AM, Dave Page  wrote:

> Hi
>
> On Tue, Jun 13, 2017 at 3:25 PM, Joao Pedro De Almeida Pereira
>  wrote:
> > Hi Hackers,
> >
> > Attached you can find the patch that corrects that Bundle tests that was
> > polluting the os and subprocess modules.
> >
> > This patch reverts the commit that skipped the test.
>
> This is almost exactly what Ashesh and I came up with (along with a
> few other variations). Unfortunately, whilst all the other tests now
> pass, the second execution of the bundler test fails with:
>
> ==
> FAIL: runTest (pgadmin.utils.javascript.tests.test_javascript_bundler.
> JavascriptBundlerTestCase)
> scenario name: JavascriptBundlerTestCase
> --
> Traceback (most recent call last):
>   File "/Users/dpage/git/pgadmin4/web/pgadmin/utils/javascript/
> tests/test_javascript_bundler.py",
> line 44, in runTest
> self._bundling_succeeds()
>   File "/Users/dpage/git/pgadmin4/web/pgadmin/utils/javascript/
> tests/test_javascript_bundler.py",
> line 70, in _bundling_succeeds
> self.mockSubprocess.call.assert_called_once_with(['yarn', 'run',
> 'bundle:dev'])
>   File "/Users/dpage/.virtualenvs/pgadmin4/lib/python2.7/site-
> packages/mock/mock.py",
> line 947, in assert_called_once_with
> raise AssertionError(msg)
> AssertionError: Expected 'call' to be called once. Called 0 times.
>
> --
>
> --
> Dave Page
> Blog: http://pgsnake.blogspot.com
> Twitter: @pgsnake
>
> EnterpriseDB UK: http://www.enterprisedb.com
> The Enterprise PostgreSQL Company
>


correct-bundle-test.patch
Description: Binary data

-- 
Sent via pgadmin-hackers mailing list (pgadmin-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgadmin-hackers

[pgadmin-hackers][patch] Correct bundle test that polluted Python modules

[Joao Pedro De Almeida Pereira]

Hi Hackers,

Attached you can find the patch that corrects that Bundle tests that was
polluting the os and subprocess modules.

This patch reverts the commit that skipped the test.


Thanks
Shruti & Joao


correct-bundle-test.patch
Description: Binary data

-- 
Sent via pgadmin-hackers mailing list (pgadmin-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgadmin-hackers

Re: [pgadmin-hackers] pgAdmin 4 commit: Temporarily disable the Javascript bundler test until

[Joao Pedro De Almeida Pereira]

Hello Hackers,
We will create a new patch to solve this issue.

Thanks
Joao & Shruti

On Tue, Jun 13, 2017 at 9:37 AM, Dave Page  wrote:

> Pivotal team,
>
> Per the commit below, and the probably many messages you've likely
> seen from Jenkins, I've had to disable the JavascriptBundler test. It
> works fine if you're testing a single server, but if you have more
> than one enabled in test_config.json, then it will fail on the second
> (and I imagine subsequent) servers.
>
> Can you take a look at this please?
>
> Thanks.
>
> On Tue, Jun 13, 2017 at 2:22 PM, Dave Page  wrote:
> > Temporarily disable the Javascript bundler test until it handle more
> than one run.
> >
> > Branch
> > --
> > master
> >
> > Details
> > ---
> > https://git.postgresql.org/gitweb?p=pgadmin4.git;a=commitdiff;h=
> cb97722fc3826f94f375f5f48807a486383ee3a9
> >
> > Modified Files
> > --
> > web/pgadmin/utils/javascript/tests/test_javascript_bundler.py | 1 +
> > 1 file changed, 1 insertion(+)
> >
> >
> > --
> > Sent via pgadmin-hackers mailing list (pgadmin-hackers@postgresql.org)
> > To make changes to your subscription:
> > http://www.postgresql.org/mailpref/pgadmin-hackers
>
>
>
> --
> Dave Page
> Blog: http://pgsnake.blogspot.com
> Twitter: @pgsnake
>
> EnterpriseDB UK: http://www.enterprisedb.com
> The Enterprise PostgreSQL Company
>

Re: [pgadmin-hackers] [pgAdmin4] [PATCH] Skip CheckForViewDataTest test based on the DB version

[Joao Pedro De Almeida Pereira]

Hello Hackers,
We sent the wrong patch. Attached is the correct one.

Sorry

Shruti & Joao

On Mon, Jun 12, 2017 at 3:08 PM, Joao Pedro De Almeida Pereira <
jdealmeidapere...@pivotal.io> wrote:

> Hi Hackers,
>
> We were trying to run the feature tests against GreenPlum and one of the
> tests was failing (CheckForViewDataTest). In this test, we are creating a
> table using COLLATE, and the COLLATE command is only available starting
> in version 9.1 of Postgres, per the documentation. We decided to skip this
> test for now for all PG Versions below 9.1
>
> Thanks
> Shruti & Joao
>


skip-collate-test.patch
Description: Binary data

-- 
Sent via pgadmin-hackers mailing list (pgadmin-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgadmin-hackers

[pgadmin-hackers] [pgAdmin4] [PATCH] Skip CheckForViewDataTest test based on the DB version

[Joao Pedro De Almeida Pereira]

Hi Hackers,

We were trying to run the feature tests against GreenPlum and one of the
tests was failing (CheckForViewDataTest). In this test, we are creating a
table using COLLATE, and the COLLATE command is only available starting in
version 9.1 of Postgres, per the documentation. We decided to skip this
test for now for all PG Versions below 9.1

Thanks
Shruti & Joao


skip-collate-test.patch
Description: Binary data

-- 
Sent via pgadmin-hackers mailing list (pgadmin-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgadmin-hackers

Re: [pgadmin-hackers] [pgAdmin4][Patch]: Load module's JS files only when required

[Joao Pedro De Almeida Pereira]

Hello Dave,
By the description on the email, it looks like the process is very similar
to the Grunt process that we were talking about, but without the patch, it
is a bit hard to get more specifics.
Can you forward the patch so we can take a look at it.

We believe that every effort done to eliminate templated Javascript and CSS
files is well worth it, and should be followed.

Thanks
Joao & Sarah

On Tue, Jun 6, 2017 at 9:39 AM, Dave Page  wrote:

> Hi
>
> On Mon, Jun 5, 2017 at 10:22 AM, Surinder Kumar
>  wrote:
> > Hi Dave,
> >
> > Please find attached patch for minifying CSS files and optimize images
> using
> > Webpack.
> >
> > Steps to run:
> >
> > 1) After applying patch, go to web directory and run npm install on
> terminal
> > to install packages which are used to parse/minify CSS files.
> >
> > 2) Run npm run build which creates dist folder inside web/pgadmin/static/
> > directory to store processed images, fonts and css files.
> >
> > 3. Set DEBUG = False in config_local.py and then run python pgAdmin.py to
> > start server.
> > I kept generated main.css and overrides.css conditional in base.html to
> load
> > them only when debug mode is False
> >
> >
> > After running "npm run build", following files/directories are generated:
> >
> > 1) main.css -  about 20 vendor CSS files are packed into this file and
> more
> > importantly the paths to images are preserved.
> >
> > 2) overrides.css - it contains bootstrap.overrides.css and pgadmin.css,
> they
> > has to be packed separately and loaded after all CSS files are loaded
> > because the purpose of these files is to override the vendor or modules
> css.
> >
> > 3) img - it contains the images used in CSS files. The name of image
> files
> > can also be hashed names for caching purpose which we can use.
> >
> > 4) fonts - it contains the fonts used in fontawesome.css and other css
> > files.
> >
> > This is a simple patch to demonstrate how CSS files will be minified with
> > Webpack.
>
> I think this is a good, simple method. It handles debug v.s release,
> and of course, any plugin modules can include their own images/CSS
> without even having to worry about webpacking if installed later.
>
> > For now it minifies only vendor CSS files, I will add modules static
> files
> > in the list later on.
>
> I think we need to do that to get a better idea of the benefits. We
> also need to get some of the JS code in there as well (Ashesh should
> be able to help with that - he told me he's de-templatised a lot of
> that now).
>
> > Any thoughts on minifying template CSS files which are built dynamically
> and
> > loaded with dependency on other modules?
>
> Let's look at why they are templates. Is that required, or could they
> be made static?
>
> > Also, I looked into Flask-webpack which generates bundled assets using
> > Webpack(webpack.config.js) and provide additionally global template tags
> to
> > use in Jinja templates.
> >
> > But it might not work with latest version of Webpack as this repo is not
> > updated since last 2 years. I need to check with latest version and I
> will
> > update on this.
>
> Given how straightforward this seems to be, I'm not sure it's needed.
>
> Joao, any comments?
>
> Nice work - thanks!
>
> --
> Dave Page
> Blog: http://pgsnake.blogspot.com
> Twitter: @pgsnake
>
> EnterpriseDB UK: http://www.enterprisedb.com
> The Enterprise PostgreSQL Company
>

Re: [pgadmin-hackers] [pgAdmin4] [PATCH] History Tab rewrite in React

[Joao Pedro De Almeida Pereira]

>
> The motivation is simple - we want a solution that works for the whole
> app, can handle debug vs. release execution, pluggable modules, and
> installations in read-only directories.

With the current configuration of Grunt, all the requirements you mention
are available.
The tasks on Grunt should only be run under development or before we are
creating the installer.

The installer should pick up only the bundled Javascript and should install
it in the correct folders, this way there is no need to rewrite or process
files in read-only directories of the end user machine.

Per the other thread on the subject (that Joao suggested continuing
> discussion on), Surinder is currently looking into flask-webpack. I
> spent some time playing with grunt and some other options last week.

We should continue the discussion about flask-webpack or Grunt or any other
Build system in the other thread, but we need to have some decision because
this patch needs a build system.
If we remove Grunt from this patch we will need to execute the following
command every time the application run:

yarn run eslint pgadmin/static/jsx/**/*.jsx
pgadmin/static/js/selection/*.js regression/javascript/**/*.jsx
regression/javascript/**/*.js *.js && yarn run webpack -- --config
webpack.config.js && python web/pgAdmin4

And to run the jasmine tests:

yarn run eslint pgadmin/static/jsx/**/*.jsx
pgadmin/static/js/selection/*.js regression/javascript/**/*.jsx
regression/javascript/**/*.js *.js && yarn run karma start

And to run the feature tests:

yarn run eslint pgadmin/static/jsx/**/*.jsx
pgadmin/static/js/selection/*.js regression/javascript/**/*.jsx
regression/javascript/**/*.js *.js && yarn run webpack -- --config
webpack.config.js
&& python runtests.py

Is this a solution that is acceptable?

However; this patch is supposed to be about the history tab rewrite.
> Whatever solution we use for webpacking/transpiling/linting/minifying
> etc, it should be a standalone change as it's decidedly non-trivial.

We split this patch into 2 different commits:
1 - Add React and all the tools needed to work with it, this includes
Grunt, Webpack and ESLint
2 - Change the history tab

Do you want a single commit for each of the following?
  React (just adds the React library via yarn)
  Webpack (adds Webpack library via yarn and creates the webpack.config.js)
  ESLint (adds ESLint library via yarn, creates .eslintrc config file, and
corrects all lint errors that previously existed)
  Grunt (adds Grunt library via yarn and creates Gruntfile.js config,
creating a pipeline of the previous three libraries/tasks)
  Our change to History

Thanks
Joao & Matt

On Tue, May 30, 2017 at 10:10 AM, Dave Page  wrote:

> Hi
>
> On Tue, May 30, 2017 at 2:47 PM, Matthew Kleiman 
> wrote:
> > Hi Dave,
> >
> > We are currently using the Grunt taskrunner to run the following tasks:
> >
> > lint the javascript code
> > start javascript tests
> > invoke webpack to transpile and bundle js and jsx files
> > minify javascript
> >
> > In order to remove Grunt from the application, we will need some other
> tool
> > to execute these listed tasks.
> >
> > There are other tools available, like Gulp.js, that we could use instead
> of
> > Grunt. We would like to understand your motivation for removing Grunt so
> we
> > can find a better solution.
>
> The motivation is simple - we want a solution that works for the whole
> app, can handle debug vs. release execution, pluggable modules, and
> installations in read-only directories.
>
> Per the other thread on the subject (that Joao suggested continuing
> discussion on), Surinder is currently looking into flask-webpack. I
> spent some time playing with grunt and some other options last week.
>
> However; this patch is supposed to be about the history tab rewrite.
> Whatever solution we use for webpacking/transpiling/linting/minifying
> etc, it should be a standalone change as it's decidedly non-trivial.
>
> --
> Dave Page
> Blog: http://pgsnake.blogspot.com
> Twitter: @pgsnake
>
> EnterpriseDB UK: http://www.enterprisedb.com
> The Enterprise PostgreSQL Company
>

Re: [pgadmin-hackers] [pgAdmin4][Patch][RM_2400]: Columns with defaults set to NULL when removing contents after pasting in the edit grid

[Joao Pedro De Almeida Pereira]

Hi Surinder,
You are right, nevertheless that was not the only error we had on the flaky
tests.

Thanks
Joao & Shruti

On Fri, May 26, 2017 at 10:18 AM, Surinder Kumar <
surinder.ku...@enterprisedb.com> wrote:

> Hi Joao,
>
> Please apply patch RM_2400v2.patch first then apply Feature test cases
> patch.
>
> On May 26, 2017 7:42 PM, "Joao Pedro De Almeida Pereira" <
> jdealmeidapere...@pivotal.io> wrote:
>
> Hello Surinder,
>
> Thanks for updating the patch. After looking into the updated version, we
> found the following issues.
>
>
> The tests looked flaky. We ran the tests three times and each time we had
> timeout issues.
> It failed twice in the location mentioned below. It also failed because
> the row1 cell2 values was [null] instead of the expected [default].
>
> Traceback (most recent call last):
>   File 
> "/Users/pivotal/workspace/pgadmin4/web/pgadmin/feature_tests/view_data_dml_queries.py",
>  line 105, in runTest
> self._copy_paste_row()
>   File 
> "/Users/pivotal/workspace/pgadmin4/web/pgadmin/feature_tests/view_data_dml_queries.py",
>  line 248, in _copy_paste_row
> self._compare_cell_value(row1_cell2_xpath, "[default]")
>   File 
> "/Users/pivotal/workspace/pgadmin4/web/pgadmin/feature_tests/view_data_dml_queries.py",
>  line 147, in _compare_cell_value
> CheckForViewDataTest.TIMEOUT_STRING
>   File 
> "/Users/pivotal/.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 div element to appear
>
> ​
>
> We also noticed that the function _wait is no longer used. Maybe we can
> remove it.
>
> Thanks
> Joao & Shruti
>
> On Fri, May 26, 2017 at 9:07 AM, Surinder Kumar <
> surinder.ku...@enterprisedb.com> wrote:
>
>> Hi
>>
>> Please find updated patch.
>>
>> On Fri, May 26, 2017 at 3:15 AM, Joao Pedro De Almeida Pereira <
>> jdealmeidapere...@pivotal.io> wrote:
>>
>>> Hello Surinder,
>>>
>>> We are having issues running the tests, this is the error that we are
>>> getting:
>>>
>>>   File 
>>> "/Users/pivotal/workspace/pgadmin4/web/regression/python_test_utils/test_utils.py",
>>>  line 196, in create_table_with_query
>>> pg_cursor.execute(query)
>>> ProgrammingError: role "postgres" does not exist
>>>
>>> Traceback (most recent call last):
>>>   File 
>>> "/Users/pivotal/workspace/pgadmin4/web/regression/python_test_utils/test_utils.py",
>>>  line 196, in create_table_with_query
>>> pg_cursor.execute(query)
>>> ProgrammingError: relation "defaults_id_seq" does not exist
>>>
>>> ​
>>> ​Fixed.​
>>>
>>> There is already a function that waits for an element to be displayed on
>>> the screen. The function is: self.page.find_by_xpath
>>>
>>> In line 179 and 180, both functions do the same thing, why do we need to
>>> wait first and then wait again. Were you experiencing flakiness?
>>>
>> I have to use another instance of webDriverWait because I was getting
>> TimeoutException. I guess timeout of 10 seconds wasn't enough to search the
>> element in DOM.
>>
>>>
>>> Does _check_xss_in_view_data method checks for Cross Site Scripting?
>>>
>> ​I forgot to change the method name.​
>>
>>>
>>> Was there any reason to duplicate self.page._connects_to_server and
>>> self.page._close_query_tool?
>>>
>> ​Fixed.
>>
>> I got following exception when I used ​"self.page._close_query_tool":
>>
>>> Traceback (most recent call last):
>>> File "/Users/surinder/Documents/Projects/pgadmin4/web/pgadmin/fea
>>> ture_tests/view_data_dml_queries.py", line 125, in runTest
>>> self.page.close_query_tool()
>>> File "/Users/surinder/Documents/Projects/pgadmin4/web/regression/
>>> feature_utils/pgadmin_page.py", line 66, in close_query_tool
>>> self.driver.switch_to.frame(self.driver.find_elements_by_tag
>>> _name("iframe")[0])
>>> File "/Users/surinder/Documents/Workspaces/PEM_35/lib/python3.5/s
>>> ite-packages/selenium/webdriver/remote/switch_to.py", line 87, in frame
>>> self._driver.execute(Command.SWITCH_TO_FRAME, {'id': frame_reference})
>>> File "/Users/surinder/Documents/Workspaces/PEM

Re: [pgadmin-hackers] [pgAdmin4][Patch][RM_2400]: Columns with defaults set to NULL when removing contents after pasting in the edit grid

[Joao Pedro De Almeida Pereira]

Hello Surinder,

Thanks for updating the patch. After looking into the updated version, we
found the following issues.


The tests looked flaky. We ran the tests three times and each time we had
timeout issues.
It failed twice in the location mentioned below. It also failed because the
row1 cell2 values was [null] instead of the expected [default].

Traceback (most recent call last):
  File 
"/Users/pivotal/workspace/pgadmin4/web/pgadmin/feature_tests/view_data_dml_queries.py",
line 105, in runTest
self._copy_paste_row()
  File 
"/Users/pivotal/workspace/pgadmin4/web/pgadmin/feature_tests/view_data_dml_queries.py",
line 248, in _copy_paste_row
self._compare_cell_value(row1_cell2_xpath, "[default]")
  File 
"/Users/pivotal/workspace/pgadmin4/web/pgadmin/feature_tests/view_data_dml_queries.py",
line 147, in _compare_cell_value
CheckForViewDataTest.TIMEOUT_STRING
  File 
"/Users/pivotal/.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 div element to appear

​

We also noticed that the function _wait is no longer used. Maybe we can
remove it.

Thanks
Joao & Shruti

On Fri, May 26, 2017 at 9:07 AM, Surinder Kumar <
surinder.ku...@enterprisedb.com> wrote:

> Hi
>
> Please find updated patch.
>
> On Fri, May 26, 2017 at 3:15 AM, Joao Pedro De Almeida Pereira <
> jdealmeidapere...@pivotal.io> wrote:
>
>> Hello Surinder,
>>
>> We are having issues running the tests, this is the error that we are
>> getting:
>>
>>   File 
>> "/Users/pivotal/workspace/pgadmin4/web/regression/python_test_utils/test_utils.py",
>>  line 196, in create_table_with_query
>> pg_cursor.execute(query)
>> ProgrammingError: role "postgres" does not exist
>>
>> Traceback (most recent call last):
>>   File 
>> "/Users/pivotal/workspace/pgadmin4/web/regression/python_test_utils/test_utils.py",
>>  line 196, in create_table_with_query
>> pg_cursor.execute(query)
>> ProgrammingError: relation "defaults_id_seq" does not exist
>>
>> ​
>> ​Fixed.​
>>
>> There is already a function that waits for an element to be displayed on
>> the screen. The function is: self.page.find_by_xpath
>>
>> In line 179 and 180, both functions do the same thing, why do we need to
>> wait first and then wait again. Were you experiencing flakiness?
>>
> I have to use another instance of webDriverWait because I was getting
> TimeoutException. I guess timeout of 10 seconds wasn't enough to search the
> element in DOM.
>
>>
>> Does _check_xss_in_view_data method checks for Cross Site Scripting?
>>
> ​I forgot to change the method name.​
>
>>
>> Was there any reason to duplicate self.page._connects_to_server and
>> self.page._close_query_tool?
>>
> ​Fixed.
>
> I got following exception when I used ​"self.page._close_query_tool":
>
>> Traceback (most recent call last):
>> File "/Users/surinder/Documents/Projects/pgadmin4/web/pgadmin/fea
>> ture_tests/view_data_dml_queries.py", line 125, in runTest
>> self.page.close_query_tool()
>> File "/Users/surinder/Documents/Projects/pgadmin4/web/regression/
>> feature_utils/pgadmin_page.py", line 66, in close_query_tool
>> self.driver.switch_to.frame(self.driver.find_elements_by_tag
>> _name("iframe")[0])
>> File "/Users/surinder/Documents/Workspaces/PEM_35/lib/python3.5/s
>> ite-packages/selenium/webdriver/remote/switch_to.py", line 87, in frame
>> self._driver.execute(Command.SWITCH_TO_FRAME, {'id': frame_reference})
>> File "/Users/surinder/Documents/Workspaces/PEM_35/lib/python3.5/s
>> ite-packages/selenium/webdriver/remote/webdriver.py", line 238, in
>> execute
>> self.error_handler.check_response(response)
>> File "/Users/surinder/Documents/Workspaces/PEM_35/lib/python3.5/s
>> ite-packages/selenium/webdriver/remote/errorhandler.py", line 193, in
>> check_response
>> raise exception_class(message, screen, stacktrace)
>> selenium.common.exceptions.WebDriverException: Message: unknown error:
>> Runtime.evaluate threw exception: Error: element is not attached to the
>> page document
>> at Cache.retrieveItem (:173:17)
>> at unwrap (:293:20)
>> at unwrap (:297:19)
>> at callFunction (:343:29)
>> at apply.ELEMENT (:357:23)
>> at :358:3
>> (Session info: chrome=58.0.3029.110)
>> (Driver info: chromedriver=2.29.461585 
>> (0be2cd95f834e9ee7c4

Re: [pgadmin-hackers] [pgAdmin4][Patch][RM_2400]: Columns with defaults set to NULL when removing contents after pasting in the edit grid

[Joao Pedro De Almeida Pereira]

Hello Surinder,

We are having issues running the tests, this is the error that we are
getting:

  File 
"/Users/pivotal/workspace/pgadmin4/web/regression/python_test_utils/test_utils.py",
line 196, in create_table_with_query
pg_cursor.execute(query)
ProgrammingError: role "postgres" does not exist

Traceback (most recent call last):
  File 
"/Users/pivotal/workspace/pgadmin4/web/regression/python_test_utils/test_utils.py",
line 196, in create_table_with_query
pg_cursor.execute(query)
ProgrammingError: relation "defaults_id_seq" does not exist

​

There is already a function that waits for an element to be displayed on
the screen. The function is: self.page.find_by_xpath

In line 179 and 180, both functions do the same thing, why do we need to
wait first and then wait again. Were you experiencing flakiness?

Does _check_xss_in_view_data method checks for Cross Site Scripting?

Was there any reason to duplicate self.page._connects_to_server and
self.page._close_query_tool?

The method _verify_insert_data looks more or less the same code as in the
end of _copy_paste_row, should this be merged?

Thanks,
Joao & Shruti


On Thu, May 25, 2017 at 4:41 PM, Dave Page  wrote:

> Hi
>
> The tests failed on both PG 9.4 and 9.6 for me :-(
>
> ==
> ERROR: runTest (pgadmin.feature_tests.view_data_dml_queries.
> CheckForViewDataTest)
> Validate Insert, Update operations in View 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 120, in runTest
> self._verify_insert_data()
>   File "/Users/dpage/git/pgadmin4/web/pgadmin/feature_tests/
> view_data_dml_queries.py",
> line 316, in _verify_insert_data
> self._compare_cell_value(cell_xpath, config_data[str(idx)][1])
>   File "/Users/dpage/git/pgadmin4/web/pgadmin/feature_tests/
> view_data_dml_queries.py",
> line 165, in _compare_cell_value
> "Timed out waiting for element to appear"
>   File "/Users/dpage/.virtualenvs/pgadmin4/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 appear
>
> Also, Can we replace the sleep with a "wait for object" or similar?
>
> On Thu, May 25, 2017 at 6:42 AM, Surinder Kumar
>  wrote:
> > Hi
> >
> > Please find attached patch for Feature test cases.
> >
> > The approach is to create a single table 'defaults' with columns of
> various
> > types(number, text, json and boolean) and write test cases for these cell
> > types with different input values.
> >
> > Following are the test cases covered:
> >
> > 1) Add a new row, save and compare the resulted value with expected
> values
> >
> > 2) Copy/Paste row, save and compare cell data
> >
> > a) Clear cell value and escape, the cell must set to [default]
> >
> > 3) Update cell:
> >
> >a) Insert two single quotes(''), expected value is blank string
> >
> >b) Clear a cell, expected value is [null]
> >
> >c) Insert a string \’\’, expected value is ''
> >
> >d) Insert a string \\’\\’, expected value is \’\’
> >
> >    e) Insert a string ’’, expected value is \\’\\’
> >
> >f)  If a checkbox cell is double clicked, return value must be 'true'
> >
> >
> > Thanks
> > Surinder Kumar
> >
> >
> >
> >
> >
> >
> > On Fri, May 19, 2017 at 6:08 PM, Surinder Kumar
> >  wrote:
> >>
> >> Hi
> >>
> >> Please find updated patch and review.
> >>
> >> On Thu, May 18, 2017 at 7:36 PM, Joao Pedro De Almeida Pereira
> >>  wrote:
> >>>
> >>> Hello Hackers,
> >>>
> >>> We reviewed the PR, and there are a lot of new lines of code in this
> >>> patch, are we sure that we can have a good coverage of the
> functionality
> >>> just by doing feature tests?
> >>> Adding more code to a 3.5k+ lines file doesn't look like a good option,
> >>> do you think it is possible to extract some of the functionality to
> their
> >>> own files and have test around those functionalities?
> >>
> >> To improve the code readability, reduce code complexity and to make code
> >> testable, the code must be splitte

Re: [pgadmin-hackers] [pgAdmin4][Patch]: Load module's JS files only when required

[Joao Pedro De Almeida Pereira]

Hello


​I wrote few grunt tasks using r.js optimizer this weekend
>
>  How did it compare to webpack?

I am already starting to wonder if Grunt is the best way to do this though.
> It might be easier (though not quite as efficient) to have the Python
> module all return their static/template JS code at initialisation,
> effectively dynamically building a single packed file containing nearly
> everything.

We could. Are you saying that the app should bundle js every run? To be
fair, this is what we're currently doing with Grunt, but it feels like we
should change this for non-development use.

Yeah, though I think there are more considerations we haven't yet thought
> of.

Something we aren't clear on is the end-user use case of dropping modules
into existing installations. Is this achieved via installer?


George & Joao


On Mon, May 22, 2017 at 5:33 PM, Dave Page  wrote:

> Hi
>
> On Monday, May 22, 2017, Surinder Kumar 
> wrote:
>
>> Hi
>>
>> On Mon, May 22, 2017 at 4:22 PM, Dave Page  wrote:
>>
>>> Hi
>>>
>>> On Monday, May 22, 2017, Surinder Kumar 
>>> wrote:
>>>
 Hi

 As per pgAdmin4 design, template JS files can either be preloaded or
 load when a specific node expands (by adding for e.g.: when: 'server').

 The JS files of several modules found to be loaded when pgAdmin4 loads
 which results in increasing:

- the number of http requests
- latency(greater request time)
- pgAdmin4 load time

 *Tested on Firefox:*

 Before applying patch

- http requests - 143
- Content size - 3.4 MB
- Load time: 4.1s (onload: 524ms)

 After applying patch

- http requests: 68
- Content size: 2.1 MB
- Load time: 2.84s (onload: 481ms)

 This is great work!
>>>
>>> However (sorry!) - I'm planning on working on an alternative change on
>>> my flight in a couple of hours. Joao has broken the Grunt code out of the
>>> History tab patch for me to work with - the idea is something like:
>>>
>>> - We continue to migrate all the JS out of templates and into static
>>> files whereever possible, using the client-side translation.
>>>
>> ​Yes. It will be then easier to cover most of the JS code for minify if
>> client side ​translation is used.
>>
>
> Unfortunately, I looked at this on my flight and it looks like it'll be a
> mammoth task. Many/most JS files also currently use the url_for template
> function - so we need to figure out how to replace that with JS code first.
>
>
>>
>>> - We then have a set of Grunt tasks that will collect all the static JS,
>>> minimise it, and pack it into a single file to load at startup.
>>>
>> ​I wrote few grunt tasks using r.js optimizer this weekend which:
>>
>> 1. Finds all static JS file recursively into the modules using regex
>> pattern and then creates the tasks to minify each JS file and store its
>> 'min.js' in same directory and loads min.js when current_app.debug is False
>> else returns JS file.
>>
>
>> 2. A task to merge given list of CSS files into a single file and then
>> minify using 'grunt-contrib-cssmin' task and then insert that file with
>> 

Re: [pgadmin-hackers] [pgAdmin4] [PATCH] History Tab rewrite in React

[Joao Pedro De Almeida Pereira]

Okay

Can you try removing web/node_modules and web/yarn.lock, and running
$ yarn install
$ grunt tests

We think this might be due to a bad version of phantomjs-prebuilt.

If that still fails, can you try running grunt as "yarn run grunt tests"?

Joao and George

On Fri, May 19, 2017 at 10:27 AM, Dave Page  wrote:

> On Thu, May 18, 2017 at 4:37 PM, Matthew Kleiman 
> wrote:
> > Dave,
> >
> > Can you try running grunt tests on the command line?
> >
> > The tests task will run the eslint and karma grunt tasks, which should
> > automatically webpack the React components before running the jasmine
> tests
> > in karma.
> >
> > If it still fails, send us the output of that.
>
> (pgadmin4)piranha:web dpage$ grunt tests
> Running "eslint:target" (eslint) task
>
> Running "karma:unit" (karma) task
> 19 05 2017 15:26:13.366:ERROR [plugin]: Error during loading
> "karma-phantomjs-launcher" plugin:
>   Path must be a string. Received null
>
> webpack: Compiled successfully.
> webpack: Compiling...
> webpack: wait until bundle finished:
> webpack: wait until bundle finished:
> webpack: wait until bundle finished:
> webpack: wait until bundle finished:
> webpack: wait until bundle finished:
> webpack: wait until bundle finished:
> webpack: wait until bundle finished:
> webpack: wait until bundle finished:
> webpack: wait until bundle finished:
> webpack: wait until bundle finished:
> webpack: wait until bundle finished:
> webpack: wait until bundle finished:
> (node:66274) DeprecationWarning: loaderUtils.parseQuery() received a
> non-string value which can be problematic, see
> https://github.com/webpack/loader-utils/issues/56
> parseQuery() will be replaced with getOptions() in the next major
> version of loader-utils.
> 19 05 2017 15:26:18.887:WARN [karma]: No captured browser, open
> http://localhost:9876/
>
> webpack: Compiled successfully.
> 19 05 2017 15:26:18.895:ERROR [karma]: Found 1 load error
> Warning: Task "karma:unit" failed. Use --force to continue.
>
> Aborted due to warnings.
>
>
> :-(
>
> --
> Dave Page
> Blog: http://pgsnake.blogspot.com
> Twitter: @pgsnake
>
> EnterpriseDB UK: http://www.enterprisedb.com
> The Enterprise PostgreSQL Company
>
>
> --
> Sent via pgadmin-hackers mailing list (pgadmin-hackers@postgresql.org)
> To make changes to your subscription:
> http://www.postgresql.org/mailpref/pgadmin-hackers
>

Re: [pgadmin-hackers] [pgAdmin4][PATCH] To fix the issue with Node rename

[Joao Pedro De Almeida Pereira]

Hello Hackers,

@Dave, @Murtuza
We do understand that the changes are tightly coupled with the tree, but if
we want to be able to upgrade our tree menu in the future, we need to start
decoupling functionality and the underlaying tree.
Another step that we can take is to change our variable naming convention
because variables like _d or this.i do not communicate their purpose,
making it hard to read.


Thanks
Joao & Shruti

On Wed, May 17, 2017 at 11:05 AM, Dave Page  wrote:

>
>
> On Wed, May 17, 2017 at 11:41 AM, Murtuza Zabuawala  enterprisedb.com> wrote:
>
>> Hi Joao,
>>
>> Yes, this patch is related to browser tree issue, In this patch we have
>> fixed some issues with 'onUpdateTreeNode' function to handle some corner
>> cases for server & server-group nodes, Current code for 'onAddTreeNode',
>> 'onUpdateTreeNode', 'onRefreshTreeNode' functions for browser tree is
>> coupled with their respective inner function calls and recursive in nature
>> due to aciTree API implementation for making function calls in orderly
>> manner.
>>
>> @Ashesh,
>> Any thoughts on this?
>>
>
> I'm obviously not Ashesh, but in general, I agree with what Joao suggests
> - treeview related code should be refactored into testable modules, that
> are independent of the tree (for the most part) whenever it makes sense to
> do so, and tests added to aid future replacement of aciTree/Backbone. That
> said, if it's not feasible in a given case, then we should go ahead and fix
> the existing code.
>
> In this case, I'm leaning towards the view that this code is too tightly
> coupled with aciTree to be worth changing more than necessary. What do you
> guys think?
>
> --
> Dave Page
> Blog: http://pgsnake.blogspot.com
> Twitter: @pgsnake
>
> EnterpriseDB UK: http://www.enterprisedb.com
> The Enterprise PostgreSQL Company
>

Re: [pgadmin-hackers] Fix for RM2421 [pgAdmin4][patch]

[Joao Pedro De Almeida Pereira]

Hello Harshal,

We review the patch and have some questions:
1) Is there any particular reason to initialize variables and functions in
the same place? We believe that it would be more readable there were no
chaining of variable creation, specially if those variables are functions.
Check line:

+++ b/web/pgadmin/static/js/backform.pgadmin.js
@@ -1528,7 +1528,18 @@
   max_value = field.max,
   isValid = true,
   intPattern = new RegExp("^-?[0-9]*$"),
-  isMatched = intPattern.test(value);
+  isMatched = intPattern.test(value),
+  trigger_invalid_event = function(msg) {

​
2) The functions added in both places look very similar, can they be merged
and extracted? We are talking about the trigger_invalid_event function.

3) The following change is very similar to the trigger_invalid_event, was
there a reason not to use it?

+++ b/web/pgadmin/static/js/backform.pgadmin.js
@@ -1573,25 +1584,23 @@
 this.model.errorModel.unset(name);
 this.model.set(name, value);
 this.listenTo(this.model, "change:" + name, this.render);
-if (this.model.collection || this.model.handler) {
-  (this.model.collection || this.model.handler).trigger(
- 'pgadmin-session:model:valid', this.model,
(this.model.collection || this.model.handler)
-);
+// Check if other fields of same model are valid before
+// triggering 'session:valid' event
+if(_.size(this.model.errorModel.attributes) == 0) {
+  if (this.model.collection || this.model.handler) {
+(this.model.collection || this.model.handler).trigger(
+   'pgadmin-session:model:valid', this.model,
(this.model.collection || this.model.handler)
+  );
+  } else {
+(this.model).trigger(
+   'pgadmin-session:valid', this.model.sessChanged(), this.model
+  );
+  }

​
4) We also noticed that the following change sets look very similiar. Is
there any reason to have this code duplicated? If not this could be a good
time to refactor it.

+++ b/web/pgadmin/static/js/backform.pgadmin.js
@@ -1528,7 +1528,18 @@

@@ -1573,25 +1584,23 @@

@@ -1631,7 +1640,18 @@

@@ -1676,25 +1696,23 @@

​

Thanks
Joao & Shruti

On Thu, May 18, 2017 at 6:01 AM, Harshal Dhumal <
harshal.dhu...@enterprisedb.com> wrote:

> Hi,
>
> Please find attached patch for RM2421
>
> Issue fixed: 1. Integer/numeric Validation is not working properly.
> 2. Wrong CPU rate unit
> --
> *Harshal Dhumal*
> *Sr. Software Engineer*
>
> EnterpriseDB India: http://www.enterprisedb.com
> The Enterprise PostgreSQL Company
>
>
> --
> Sent via pgadmin-hackers mailing list (pgadmin-hackers@postgresql.org)
> To make changes to your subscription:
> http://www.postgresql.org/mailpref/pgadmin-hackers
>
>

Re: [pgadmin-hackers] [pgAdmin4][Patch][RM_2400]: Columns with defaults set to NULL when removing contents after pasting in the edit grid

[Joao Pedro De Almeida Pereira]

Hello Hackers,

We reviewed the PR, and there are a lot of new lines of code in this patch,
are we sure that we can have a good coverage of the functionality just by
doing feature tests?
Adding more code to a 3.5k+ lines file doesn't look like a good option, do
you think it is possible to extract some of the functionality to their own
files and have test around those functionalities?

Do we really need to have an epicRandomString function in our code? Would
it be better to use a library that would provide us that functionality out
of the box?
The functions this.applyValue in slick.pgadmin.editors.js that were change
in this patch are exactly the same, can we extract that code into a single
function instead of repeating the code?

Using feature tests is a good option to ensure that the integration of all
the components of the application is working as expected, but in order to
tests behaviors that are edge cases the Unit Tests are much cheaper to run
and create.

Thanks
Joao & Shruti


On Thu, May 18, 2017 at 1:22 AM, Surinder Kumar <
surinder.ku...@enterprisedb.com> wrote:

> Hi Dave,
>
> Please review the updated patch.
>
> On Wed, May 17, 2017 at 8:46 PM, Dave Page  wrote:
>
>> Hi
>>
>> On Wed, May 17, 2017 at 3:08 PM, Surinder Kumar <
>> surinder.ku...@enterprisedb.com> wrote:
>>
>>> Hi Dave,
>>>
>>> *Implementation:*
>>>
>>> 1) Took a flag 'is_row_copied' for each copied row:
>>>
>>>  - to distinguish it from add/update row.
>>>  - to write code specific to copied row only as it requires different
>>> logic than add row.
>>>
>>> 2) After pasting an existing row:
>>>
>>>  - If a user clear the cell value, it must set cell to [default] value
>>> if default value is explicitly given for column while creating table
>>> otherwise [null].
>>>
>>>  - Again, if a user entered a value in same cell, then removes that
>>> value, set it to [null] (the same behaviour is for add row).
>>>
>>> 3) Took a 2-dimensional array "grid.copied_rows" to keep track the
>>> changes of each row's cell so that changes made to each cell are
>>> independent and removed once data is saved.
>>>
>>> 4) On pasting a row, the cell must be highlighted with light green
>>> colour, so triggers an addNewRow event. Now copied row will add to grid
>>> instead of re-rendering all rows again.
>>>
>>> 5) Moved the common logic into functions.
>>>
>>> This patch pass the feature test cases and jasmine test case.
>>> Also I will add the test cases for copy row and send an updated patch.
>>>
>>> Please find attached patch and review.
>>>
>>
>> Looks good. I saw the following issues:
>>
>> - The "new" row is not available once I've created the first new row, or
>> pasted some.
>>
> ​Fixed.
>
>>
>> - Rows are pasted in reverse order.
>>
> ​Fixed.
>
>>
>> - If I copy/paste a new row that has yet to be saved, none of the values
>> are actually copied.
>>
> ​Fixed.​
>
>>
>> - Attempting to save a row that contains all null/default values results
>> in an invalid query:
>>
>> INSERT INTO public.defaults (
>> ) VALUES (
>> );
>>
>> I think the only answer here is to explicitly insert NULL into any
>> columns *without* a default value.
>>
> ​I could not reproduce, However  #3 might have fixed it too.​
>
> Apart from this, I noticed epicRandomString(...) function doesn't generate
> unique number always, due to this save and delete rows was affected. Not
> all rows saved or deleted. The new function always returns a unique random
> number.
> Fixed.
>
>>
>> Thanks.
>>
>> --
>> Dave Page
>> Blog: http://pgsnake.blogspot.com
>> Twitter: @pgsnake
>>
>> EnterpriseDB UK: http://www.enterprisedb.com
>> The Enterprise PostgreSQL Company
>>
>
>
>
> --
> Sent via pgadmin-hackers mailing list (pgadmin-hackers@postgresql.org)
> To make changes to your subscription:
> http://www.postgresql.org/mailpref/pgadmin-hackers
>
>

Re: [pgadmin-hackers] [pgAdmin4][PATCH] To fix the issue with Node rename

[Joao Pedro De Almeida Pereira]

Hello Hackers,

We looked into this patch and it looks like it touches part of the tree
menu am I correct?
In our previous conversations with Dave, he mentioned that he wanted to
replace the tree menu with a more stable version of it. In our perspective,
every time that we have the opportunity extract functions that are relevant
to the tree, we should do it and also wrap them with tests so that in the
future, when the community starts the work of upgrading the tree we could
have some confidence that our changes are not breaking any functionality.
Also removing as much code from template files as possible should also be a
goal that we aim for, because that code becomes testable.

Thanks
Joao

On Tue, May 16, 2017 at 8:36 AM, Harshal Dhumal <
harshal.dhu...@enterprisedb.com> wrote:

> Hi,
>
> Here is updated patch to fix issue with node rename. In this patch I have
> fixed issue which I mentioned earlier in this mail thread.
>
> @Murtuza, as you know all other scenarios, can you please test this patch
> once.
>
> Thanks,
>
>
>
> --
> *Harshal Dhumal*
> *Sr. Software Engineer*
>
> EnterpriseDB India: http://www.enterprisedb.com
> The Enterprise PostgreSQL Company
>
> On Mon, May 15, 2017 at 2:46 PM, Dave Page  wrote:
>
>> OK, that needs fixing then...
>>
>>
>> On Mon, May 15, 2017 at 9:59 AM, Harshal Dhumal <
>> harshal.dhu...@enterprisedb.com> wrote:
>>
>>> Hi Dave,
>>>
>>>
>>> --
>>> *Harshal Dhumal*
>>> *Sr. Software Engineer*
>>>
>>> EnterpriseDB India: http://www.enterprisedb.com
>>> The Enterprise PostgreSQL Company
>>>
>>> On Mon, May 15, 2017 at 1:43 PM, Dave Page  wrote:
>>>
 So is the last patch considered good?

>>> Except one thing, when user renames server group (note that server group
>>> should never be expanded before renaming) and then expands it by clicking +
>>> icon then it's old name gets restored.
>>>
>>> Step:
>>> 1. Load application in browser.
>>> 2. Do not expand any of the server group.
>>> 3. Rename Server group (new name gets updated in tree).
>>> 4. Now expand Server group (old name gets restored in tree).
>>>
>>> Note that this happens only when Server groups was never expanded before.
>>>
>>>
>>>

 On Mon, May 15, 2017 at 9:11 AM, Murtuza Zabuawala <
 murtuza.zabuaw...@enterprisedb.com> wrote:

> Hi Dave,
>
> Yes, It is existing one only, We did not touch on any part of sorting
> algorithm in this patch.
>
> --
> Regards,
> Murtuza Zabuawala
> EnterpriseDB: http://www.enterprisedb.com
> The Enterprise PostgreSQL Company
>
> On Mon, May 15, 2017 at 1:24 PM, Dave Page  wrote:
>
>> Ashesh is out this week. As long as new nodes are sorted with the
>> same algorithm as existing ones, that's fine.
>>
>> On Mon, May 15, 2017 at 8:48 AM, Murtuza Zabuawala <
>> murtuza.zabuaw...@enterprisedb.com> wrote:
>>
>>> Hi Harshal,
>>>
>>> We are using https://github.com/javve/natural-sort for sorting
>>> nodes which is implemented by Ashesh.
>>>
>>> @Ashesh,
>>> Any suggestion on this?
>>>
>>> --
>>> Regards,
>>> Murtuza Zabuawala
>>> EnterpriseDB: http://www.enterprisedb.com
>>> The Enterprise PostgreSQL Company
>>>
>>> On Mon, May 15, 2017 at 1:07 PM, Harshal Dhumal <
>>> harshal.dhu...@enterprisedb.com> wrote:
>>>
 Hi Murtuza,

 Currently nodes are sorted in case sensitive manner it should
 be case insensitive.



 See current Server group order is A, Servers, a1, a​2. It should
 be A, a1, a2, Servers.
 Similarly check sorting order for server and database nodes


 --
 *Harshal Dhumal*
 *Sr. Software Engineer*

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

 On Fri, May 12, 2017 at 7:08 PM, Murtuza Zabuawala <
 murtuza.zabuaw...@enterprisedb.com> wrote:

> Hi Ashesh,
>
> Please find updated patch as discussed.
>
> --
> Regards,
> Murtuza Zabuawala
> EnterpriseDB: http://www.enterprisedb.com
> The Enterprise PostgreSQL Company
>
> On Fri, May 12, 2017 at 11:37 AM, Murtuza Zabuawala <
> murtuza.zabuaw...@enterprisedb.com> wrote:
>
>> Hi Ashesh,
>>
>> As discussed please find updated patch removing hardcoded check
>> for server & server-group node.
>>
>> --
>> Regards,
>> Murtuza Zabuawala
>> EnterpriseDB: http://www.enterprisedb.com
>> The Enterprise PostgreSQL Company
>>
>> On Fri, Apr 28, 2017 at 1:29 PM, Murtuza Zabuawala <
>> murtuza.zabuaw...@enterprisedb.com> wrote:
>>
>>> Hi Ashesh,
>>>
>>> PFA updated patch for the issue.
>>>
>>> --
>>> Regards,
>>>

Re: [pgadmin-hackers] Re: [pgAdmin4][Patch][RM2399]: Disabled row's background color disappeared on save in View data

[Joao Pedro De Almeida Pereira]

We were commenting on the disabled state of the copy button, so nevermind/
we are currently working on an update to the copy functionality of the grid.
Thanks for the fixes!

On Mon, May 15, 2017 at 1:31 PM, Surinder Kumar <
surinder.ku...@enterprisedb.com> wrote:

> Hi,
>
> The array temp_new_rows is used to keep track to new added rows in View
> data. It is used to add 'disabled_row'class for new rows to highlight
> when grid renders after delete/copy/add row operation.
>
> Please find attached patch.
>
> On Mon, May 15, 2017 at 10:14 PM, Surinder Kumar <
> surinder.ku...@enterprisedb.com> wrote:
>
>> Hi Joao,
>>
>> The single cell selection works for me using Cmd+C. Not reproducible for
>> me.
>>
>> Please provide steps to reproduce.
>>
>>
>>
>>
>> On Mon, May 15, 2017 at 9:58 PM, Harshal Dhumal <
>> harshal.dhu...@enterprisedb.com> wrote:
>>
>>> Hi,
>>>
>>> Issue caused because variable temp_new_rows was access before it was
>>> initialised. Attached patch fixes this issue.
>>>
>>> --
>>> *Harshal Dhumal*
>>> *Sr. Software Engineer*
>>>
>>> EnterpriseDB India: http://www.enterprisedb.com
>>> The Enterprise PostgreSQL Company
>>>
>>> On Mon, May 15, 2017 at 9:28 PM, Dave Page  wrote:
>>>
>>>> Aww, nuts. I thought we had tests for that? Did we miss that particular
>>>> case?
>>>>
>>>> Anyway, copying of a single value still works (with Cmd+C), so I'm not
>>>> inclined to re-wrap everything for this unless someone can get a patch to
>>>> me by ~9AM UK time tomorrow. Surinder?
>>>>
>>>> On Mon, May 15, 2017 at 4:50 PM, Joao Pedro De Almeida Pereira <
>>>> jdealmeidapere...@pivotal.io> wrote:
>>>>
>>>>> Hi hackers,
>>>>>
>>>>> We were checking out the selection functionality after this
>>>>> commit--looks like selection no longer works for single cells.
>>>>>
>>>>> If you press a cell in the grid the following message is console
>>>>> logged:
>>>>>
>>>>> sqleditor.js:869 Uncaught TypeError: Cannot read property 'indexOf' of 
>>>>> undefined
>>>>> at SlickGrid. (sqleditor.js:869)
>>>>> at Event.notify (slick.core.js:143)
>>>>> at trigger (slick.grid.js:1067)
>>>>> at setActiveCellInternal (slick.grid.js:2693)
>>>>> at HTMLDivElement.handleClick (slick.grid.js:2469)
>>>>> at HTMLDivElement.dispatch (jquery-1.11.2.js:4665)
>>>>> at HTMLDivElement.$event.dispatch (jquery.event.drag-2.2.js:374)
>>>>> at HTMLDivElement.elemData.handle (jquery-1.11.2.js:4333)
>>>>>
>>>>> ​
>>>>>
>>>>> Thanks
>>>>> George & Joao
>>>>>
>>>>> On Mon, May 15, 2017 at 10:05 AM, Dave Page  wrote:
>>>>>
>>>>>> I've committed a modified version of this patch following some
>>>>>> discussion on IM with Surinder.
>>>>>>
>>>>>> Thanks Surinder!
>>>>>>
>>>>>> On Mon, May 15, 2017 at 10:56 AM, Surinder Kumar <
>>>>>> surinder.ku...@enterprisedb.com> wrote:
>>>>>>
>>>>>>> Hi
>>>>>>>
>>>>>>> The regression test cases for 'copy row' was failing.
>>>>>>> This patch was assuming that each selected row will have primary key
>>>>>>> due to selection was not working. Fixed.
>>>>>>>
>>>>>>> Please find revised patch.
>>>>>>>
>>>>>>> Thanks
>>>>>>> Surinder Kumar
>>>>>>>
>>>>>>> On Mon, May 15, 2017 at 1:13 PM, Surinder Kumar <
>>>>>>> surinder.ku...@enterprisedb.com> wrote:
>>>>>>>
>>>>>>>> Hi,
>>>>>>>>
>>>>>>>> This patch contains following changes:
>>>>>>>>
>>>>>>>> 1) When a new row is added, allow to delete if changes are not
>>>>>>>> saved.
>>>>>>>>
>>>>>>>> 2) Disable new row selection if primary key is not given but
>>>>>>>> changes are saved on server.
>>>>>>>>
>>>>>>>> 3) Copy and paste one or more rows, then add new row doesn't
>>>>>>>> work(entered value doesn't appear).
>>>>>>>>
>>>>>>>> 4) After deleting a row, add new row doesn't work.
>>>>>>>>
>>>>>>>> 5) New row added with explicitly given primary key should not
>>>>>>>> disabled.
>>>>>>>>
>>>>>>>>
>>>>>>>> Please review.
>>>>>>>>
>>>>>>>> Thanks,
>>>>>>>> Surinder Kumar
>>>>>>>>
>>>>>>>
>>>>>>>
>>>>>>>
>>>>>>> --
>>>>>>> Sent via pgadmin-hackers mailing list (pgadmin-hack...@postgresql.or
>>>>>>> g)
>>>>>>> To make changes to your subscription:
>>>>>>> http://www.postgresql.org/mailpref/pgadmin-hackers
>>>>>>>
>>>>>>>
>>>>>>
>>>>>>
>>>>>> --
>>>>>> Dave Page
>>>>>> Blog: http://pgsnake.blogspot.com
>>>>>> Twitter: @pgsnake
>>>>>>
>>>>>> EnterpriseDB UK: http://www.enterprisedb.com
>>>>>> The Enterprise PostgreSQL Company
>>>>>>
>>>>>
>>>>>
>>>>
>>>>
>>>> --
>>>> Dave Page
>>>> Blog: http://pgsnake.blogspot.com
>>>> Twitter: @pgsnake
>>>>
>>>> EnterpriseDB UK: http://www.enterprisedb.com
>>>> The Enterprise PostgreSQL Company
>>>>
>>>
>>>
>>
>

Re: [pgadmin-hackers] Re: [pgAdmin4][Patch][RM2399]: Disabled row's background color disappeared on save in View data

[Joao Pedro De Almeida Pereira]

Hi hackers,

We were checking out the selection functionality after this commit--looks
like selection no longer works for single cells.

If you press a cell in the grid the following message is console logged:

sqleditor.js:869 Uncaught TypeError: Cannot read property 'indexOf' of undefined
at SlickGrid. (sqleditor.js:869)
at Event.notify (slick.core.js:143)
at trigger (slick.grid.js:1067)
at setActiveCellInternal (slick.grid.js:2693)
at HTMLDivElement.handleClick (slick.grid.js:2469)
at HTMLDivElement.dispatch (jquery-1.11.2.js:4665)
at HTMLDivElement.$event.dispatch (jquery.event.drag-2.2.js:374)
at HTMLDivElement.elemData.handle (jquery-1.11.2.js:4333)

​

Thanks
George & Joao

On Mon, May 15, 2017 at 10:05 AM, Dave Page  wrote:

> I've committed a modified version of this patch following some discussion
> on IM with Surinder.
>
> Thanks Surinder!
>
> On Mon, May 15, 2017 at 10:56 AM, Surinder Kumar <
> surinder.ku...@enterprisedb.com> wrote:
>
>> Hi
>>
>> The regression test cases for 'copy row' was failing.
>> This patch was assuming that each selected row will have primary key due
>> to selection was not working. Fixed.
>>
>> Please find revised patch.
>>
>> Thanks
>> Surinder Kumar
>>
>> On Mon, May 15, 2017 at 1:13 PM, Surinder Kumar <
>> surinder.ku...@enterprisedb.com> wrote:
>>
>>> Hi,
>>>
>>> This patch contains following changes:
>>>
>>> 1) When a new row is added, allow to delete if changes are not saved.
>>>
>>> 2) Disable new row selection if primary key is not given but changes are
>>> saved on server.
>>>
>>> 3) Copy and paste one or more rows, then add new row doesn't
>>> work(entered value doesn't appear).
>>>
>>> 4) After deleting a row, add new row doesn't work.
>>>
>>> 5) New row added with explicitly given primary key should not disabled.
>>>
>>>
>>> Please review.
>>>
>>> Thanks,
>>> Surinder Kumar
>>>
>>
>>
>>
>> --
>> Sent via pgadmin-hackers mailing list (pgadmin-hackers@postgresql.org)
>> To make changes to your subscription:
>> http://www.postgresql.org/mailpref/pgadmin-hackers
>>
>>
>
>
> --
> Dave Page
> Blog: http://pgsnake.blogspot.com
> Twitter: @pgsnake
>
> EnterpriseDB UK: http://www.enterprisedb.com
> The Enterprise PostgreSQL Company
>

Re: [pgadmin-hackers] Re: Server side cursor limitations for on demand loading of data in query tool [RM2137] [pgAdmin4]

[Joao Pedro De Almeida Pereira]

We were only able to apply the patch on 1f903ba2 (were seeing patch does
not apply due to sqleditor.js conflicts)
The javascript tests passed, but we were unable to copy rows or columns or
cells when running the application. Could you run feature tests?

Now that more functionality is being added to sqleditor.js, this may be a
good time to extract the functionality to separate files. This will
increase readability, and encourage separation of concerns. It will also
make changes easier to test in isolation.

It's probably a good idea to test the changes made to the python as well as
javascript code. In this case, the new behavior of poll() in sqleditor
__init__ should be tested.

--Joao and George


On Fri, May 12, 2017 at 8:32 AM, Harshal Dhumal <
harshal.dhu...@enterprisedb.com> wrote:

> Hi,
>
> Here is rebased patch.
>
> --
> *Harshal Dhumal*
> *Sr. Software Engineer*
>
> EnterpriseDB India: http://www.enterprisedb.com
> The Enterprise PostgreSQL Company
>
> On Fri, May 12, 2017 at 1:28 PM, Harshal Dhumal <
> harshal.dhu...@enterprisedb.com> wrote:
>
>> Hi,
>>
>> Pls find updated patch for on demand loading feature.
>>
>> Changes:
>> 1. Added row number.
>> 2. load renaming result set if user performs select all operation either
>> on grid or column.
>> 3. Fixed Row selection issue and row past issue.
>> 4. Updated existing jasmine test cases.
>>
>>
>> --
>> *Harshal Dhumal*
>> *Sr. Software Engineer*
>>
>> EnterpriseDB India: http://www.enterprisedb.com
>> The Enterprise PostgreSQL Company
>>
>> On Tue, May 9, 2017 at 10:02 PM, Harshal Dhumal <
>> harshal.dhu...@enterprisedb.com> wrote:
>>
>>> Hi,
>>>
>>> --
>>> *Harshal Dhumal*
>>> *Sr. Software Engineer*
>>>
>>> EnterpriseDB India: http://www.enterprisedb.com
>>> The Enterprise PostgreSQL Company
>>>
>>> On Tue, May 9, 2017 at 1:08 AM, Sarah McAlear 
>>> wrote:
>>>
 Hi Harshal!

 We applied your patch and ran the Javascript tests and realized there
 are 6 tests failing. After that we replicate the failures by hand and found
 the following issues:

 - Clicking in the checkboxes of the rows does not select the row
 anymore.
 - When copying and pasting (any) data, we realized that it copied an
 empty dataset.

 Thanks for pointing out these issues. I have fixed them at my end and
>>> also remaining tests are failing because in earlier implementation of sql
>>> editor we have used pain 2Darray to provide data to slick grid and In this
>>> patch I have used Slick grid DataView
>>>  to provide data
>>> to slick gird. I'll need to update test cases to support this DataView
>>> change. I'll send updated patch with all of the above changes along with
>>> suggestion given by Dave.
>>>
>>>
>>>
 We didn't review the patch further because there aren't any tests for
 the newly implemented functionality.

 Is there a place to find a dataset with 96k rows that you guys were
 testing with?

>>>
>>> Execute below SQL queries to create table dummy_data  with dataset of
>>> 100k records. (you can change number of records to generate by replacing
>>> 10 in below SQL query)
>>>
>>>
>>>
>>> *CREATE TABLE public.dummy_data*
>>> *(*
>>> *   id serial NOT NULL, *
>>> *   name text*
>>> *) *
>>> *WITH (*
>>> *  OIDS = FALSE*
>>> *)*
>>> *;*
>>> *ALTER TABLE public.dummy_data*
>>> *  OWNER TO postgres;*
>>>
>>>
>>> *INSERT INTO public.dummy_data(*
>>> *id, name)*
>>> * SELECT generate_series(1,10), 'dummy name';*
>>>
>>>
>>>
>>>
>>>
>>>
>>>

 Note: To run the Javascript test you can use the following commands
 ```
 $ cd web
 $ yarn install
 $ yarn run karma start --single-run
 ```

 Thanks
 Joao & Sarah

 On Mon, May 8, 2017 at 9:16 AM, Harshal Dhumal <
 harshal.dhu...@enterprisedb.com> wrote:

> Hi Dave,
>
> --
> *Harshal Dhumal*
> *Sr. Software Engineer*
>
> EnterpriseDB India: http://www.enterprisedb.com
> The Enterprise PostgreSQL Company
>
> On Mon, May 8, 2017 at 4:49 PM, Dave Page  wrote:
>
>> Hi
>>
>> This is fantastic! The speedup in the query tool is phenomenal with
>> large result sets. For 96K rows, I'm now seeing 1 second until I can 
>> browse
>> the data, vs. 13 in v1.4 (and 22 secs in pgAdmin 3)!! Awesome work 
>> Harshal
>> :-)
>>
>> Questions/comments:
>>
>> - Can we put the row number in the left-hand column, to the right of
>> the checkbox? This patch highlights just how much value that had in 
>> pgAdmin
>> 3 (I hadn't realised we had missed it until now)
>>
>> - If the user clicks the checkbox to select all rows, we need to
>> retrieve them all at that time, otherwise they may be inadvertently 
>> working
>> with a truncated result set.
>>
>> Sure I'll do above both the changes.
>
>
>> - Are any changes needed to 

Re: [pgadmin-hackers][patch] Dependents and Dependencies in GreenPlum

[Joao Pedro De Almeida Pereira]

Hello Akshay,

You can find attached a new version of the patch that changes the test in
question.
This new version instead of requiring a creation of a new Role now does the
following:
1 - Creates a new Role
2 - Creates a table with that Role
3 - Checks for dependencies

Thanks
Joao & George

On Thu, May 11, 2017 at 7:33 AM, Joao Pedro De Almeida Pereira <
jdealmeidapere...@pivotal.io> wrote:

> Hello all,
> This test is trying to check if the sql to retrieve the role dependencies
> of a object in the database is correct. Using a different user to connect
> to the database and create tables there is no need for extra setup because
> the role depedencies table entry is added automatically. Also we talked
> with some internal users and they told us they always used a different user
> to connect to the database with pgadmin.
>
> After your comments we realize that the test need to have a different
> setup in order to become more readable and resilient. What do we need to do
> in the test setup to ensure an entry is added to the role dependency table?
>
> Also the test need to be a little bit more resilient to work independently
> of the user that is used on the test.
>
> Thanks
> Joao
>
> On Thu, May 11, 2017, 3:20 AM Khushboo Vashi  com> wrote:
>
>> On Thu, May 11, 2017 at 12:03 PM, Dave Page  wrote:
>>
>>>
>>>
>>> On 11 May 2017, at 07:11, Akshay Joshi 
>>> wrote:
>>>
>>> Hi Sarah
>>>
>>> On Thu, May 11, 2017 at 2:30 AM, Sarah McAlear 
>>> wrote:
>>>
>>>> Hi Ashkay!
>>>>
>>>> TestTablesNodeSql hasn't failed but TestRoleDependenciesSql failed with
>>>>> below error:
>>>>> .\test_role_dependencies_sql.py\", line 41, in assertions\n
>>>>>  self.assertEqual(1, len(fetch_result))\nAssertionError: 1 != 0
>>>>
>>>>
>>>> We experienced a similar problem when we started developing this patch.
>>>> We noticed that we were connecting to the database with the super user (the
>>>> db owner user), which did not create the role we are expecting when a table
>>>> is created. To ensure that this role is created, we had to create a new
>>>> user and use it to execute all the tests. After we made this change, the
>>>> tests passed.
>>>>
>>>> Can you try to create a new user and use it to execute your tests?
>>>>
>>>
>>> I did that and it works. I have created new user 'test' with
>>> superuser privileges and update parameter db_username="test" in
>>> test_config.json file. But still I am unable to understand why it doesn't
>>> work with 'postgres' (default) user?
>>>
>>>
>>> Agreed. That suggests something fishy is going on that should be
>>> understood.
>>>
>>>
>> We don't check role dependencies for every object. As per the code, the
>> role dependencies are executed for columns not for the table itself.
>> And in the test cases, the table object is trying to execute the role
>> dependencies, so I think this test case is not correct.
>>
>>>
>>>> Thanks,
>>>> João & Sarah
>>>>
>>>> On Wed, May 10, 2017 at 1:56 PM, Akshay Joshi <
>>>> akshay.jo...@enterprisedb.com> wrote:
>>>>
>>>>>
>>>>>
>>>>> On May 10, 2017 19:07, "Sarah McAlear"  wrote:
>>>>>
>>>>> Hi Akshay!
>>>>>
>>>>> Does this error occur on 9.6 or 10.0? We tested it in 9.6 and all our
>>>>> tests pass.
>>>>>
>>>>>
>>>>> On both 9.6 and 10.0. Query returned 0 rows which is compared
>>>>> against 1 in assert statement.
>>>>>
>>>>>
>>>>> Thanks!
>>>>> João & Sarah
>>>>>
>>>>> On Wed, May 10, 2017 at 2:29 AM, Akshay Joshi <
>>>>> akshay.jo...@enterprisedb.com> wrote:
>>>>>
>>>>>> Hi Sarah
>>>>>>
>>>>>> On Tue, May 9, 2017 at 9:03 PM, Sarah McAlear 
>>>>>> wrote:
>>>>>>
>>>>>>> Hi Akshay!
>>>>>>>
>>>>>>>
>>>>>>>> Some test file names ended with "*_sql_template.py*" do we need to
>>>>>>>> add that string ?
>>>>>>>
>>>>>>> we added this suffix af

Re: [pgadmin-hackers] [pgAdmin4][PATCH] To fix the issue of menu visibility when node is hidden

[Joao Pedro De Almeida Pereira]

Hi Murtuza,

Since you are adding a new function to the browser.js component, we would
encourage you to extract the function out of the templated javascript file
so that it can be tested. A good example of this would be the
pgadmin/static/js/size_prettify.js.

For this specific function we found one function that is used from
browser.js, and you can Stub that out using
createSpyObject(browser, 'get_preference');

We think the object here is not 100% line coverage but 100% of the behavior
should be covered.

Also, suggest you change the variable name perf to preference. Looks like
it is a typo.

Regards,
João & Matt

On Thu, May 11, 2017 at 3:08 AM, Murtuza Zabuawala <
murtuza.zabuaw...@enterprisedb.com> wrote:

> Hi,
>
> PFA patch to fix the issue where user hides the any node from Preference
> dialog but the menu(both context/object) still appears in pgAdmin4.
> RM#2225
>
> --
> Regards,
> Murtuza Zabuawala
> EnterpriseDB: http://www.enterprisedb.com
> The Enterprise PostgreSQL Company
>
>
> --
> Sent via pgadmin-hackers mailing list (pgadmin-hackers@postgresql.org)
> To make changes to your subscription:
> http://www.postgresql.org/mailpref/pgadmin-hackers
>
>

Re: [pgadmin-hackers][patch] Dependents and Dependencies in GreenPlum

[Joao Pedro De Almeida Pereira]

Hello all,
This test is trying to check if the sql to retrieve the role dependencies
of a object in the database is correct. Using a different user to connect
to the database and create tables there is no need for extra setup because
the role depedencies table entry is added automatically. Also we talked
with some internal users and they told us they always used a different user
to connect to the database with pgadmin.

After your comments we realize that the test need to have a different setup
in order to become more readable and resilient. What do we need to do in
the test setup to ensure an entry is added to the role dependency table?

Also the test need to be a little bit more resilient to work independently
of the user that is used on the test.

Thanks
Joao

On Thu, May 11, 2017, 3:20 AM Khushboo Vashi <
khushboo.va...@enterprisedb.com> wrote:

> On Thu, May 11, 2017 at 12:03 PM, Dave Page  wrote:
>
>>
>>
>> On 11 May 2017, at 07:11, Akshay Joshi 
>> wrote:
>>
>> Hi Sarah
>>
>> On Thu, May 11, 2017 at 2:30 AM, Sarah McAlear 
>> wrote:
>>
>>> Hi Ashkay!
>>>
>>> TestTablesNodeSql hasn't failed but TestRoleDependenciesSql failed with
 below error:
 .\test_role_dependencies_sql.py\", line 41, in assertions\n 
 self.assertEqual(1,
 len(fetch_result))\nAssertionError: 1 != 0
>>>
>>>
>>> We experienced a similar problem when we started developing this patch.
>>> We noticed that we were connecting to the database with the super user (the
>>> db owner user), which did not create the role we are expecting when a table
>>> is created. To ensure that this role is created, we had to create a new
>>> user and use it to execute all the tests. After we made this change, the
>>> tests passed.
>>>
>>> Can you try to create a new user and use it to execute your tests?
>>>
>>
>> I did that and it works. I have created new user 'test' with
>> superuser privileges and update parameter db_username="test" in
>> test_config.json file. But still I am unable to understand why it doesn't
>> work with 'postgres' (default) user?
>>
>>
>> Agreed. That suggests something fishy is going on that should be
>> understood.
>>
>>
> We don't check role dependencies for every object. As per the code, the
> role dependencies are executed for columns not for the table itself.
> And in the test cases, the table object is trying to execute the role
> dependencies, so I think this test case is not correct.
>
>>
>>> Thanks,
>>> João & Sarah
>>>
>>> On Wed, May 10, 2017 at 1:56 PM, Akshay Joshi <
>>> akshay.jo...@enterprisedb.com> wrote:
>>>


 On May 10, 2017 19:07, "Sarah McAlear"  wrote:

 Hi Akshay!

 Does this error occur on 9.6 or 10.0? We tested it in 9.6 and all our
 tests pass.


 On both 9.6 and 10.0. Query returned 0 rows which is compared
 against 1 in assert statement.


 Thanks!
 João & Sarah

 On Wed, May 10, 2017 at 2:29 AM, Akshay Joshi <
 akshay.jo...@enterprisedb.com> wrote:

> Hi Sarah
>
> On Tue, May 9, 2017 at 9:03 PM, Sarah McAlear 
> wrote:
>
>> Hi Akshay!
>>
>>
>>> Some test file names ended with "*_sql_template.py*" do we need to
>>> add that string ?
>>
>> we added this suffix after moving the tests up a level to
>> tables/tests to clarify what subject they applied to. we changed the 
>> suffix
>> to "_sql.py"
>>
>> - Files "test_column_acl_sql_template.py" and "
>>> test_column_properties_sql_template.py" should be moved from
>>> tables->tests to tables->column->tests. As it's related to column.
>>> - Files "test_trigger_get_oid_sql_template.py" and "
>>> test_trigger_nodes_sql_template.py" should be moved from
>>> tables->tests to tables->triggers->tests. As its related to triggers.
>>
>> these tests are related to the sql files in tables/templates/column,
>> not tables/column, so moving them into tables/column would be more
>> confusing.
>>
>> Following test cases are failing
>>
>> Thank you, fixed, see new patch. Can you confirm that
>> TestTablesNodeSql doesn't fail in your environment?
>>
>
> TestTablesNodeSql hasn't failed but TestRoleDependenciesSql failed
> with below error:
> .\test_role_dependencies_sql.py\", line 41, in assertions\n
> self.assertEqual(1, len(fetch_result))\nAssertionError: 1 != 0
>
>
>> Thanks,
>> George & Sarah
>>
>>
>
>
> --
> *Akshay Joshi*
> *Principal Software Engineer *
>
>
>
> *Phone: +91 20-3058-9517 <+91%2020%203058%209517>Mobile: +91
> 976-788-8246 <+91%2097678%2088246>*
>



>>>
>>
>>
>> --
>> *Akshay Joshi*
>> *Principal Software Engineer *
>>
>>
>>
>> *Phone: +91 20-3058-9517Mobile: +91 976-788-8246*
>>
>>

[pgadmin-hackers] Running feature tests in CI

[Joao Pedro De Almeida Pereira]

Hello Hackers,

We were looking through the CI configuration and we did not understand why
the feature tests are not running. What is the reason for excluding them?

Thanks
Joao & Sarah

Re: [pgadmin-hackers] [pgAdmin4][Patch]: Fixed RM #2315 : Sorting by size is broken

[Joao Pedro De Almeida Pereira]

Hi Khushboo,

We looked at your updated patch:
- the tests look good!
- there's a small comment header change needed in size_prettify_spec
- it looks like the previous and new functions have different behaviors
(where the new behavior changes units on 1 of the lower unit, a GB)
- We should clarify that in size_prettify, we were mostly talking about
name readability in your first patch, and that the original structure was
better (especially the sizes array)
At first glance, the new sizePrettify appears to behave like a for loop, so
that might be the simplest refactor.

Thanks,
Joao and George



On Thu, May 4, 2017 at 5:02 AM, Khushboo Vashi <
khushboo.va...@enterprisedb.com> wrote:

> Hi,
>
> Please find the attached updated patch.
>
> Thanks,
> Khushboo
>
> On Fri, Apr 28, 2017 at 7:58 PM, Matthew Kleiman 
> wrote:
>
>> Hi Khushboo,
>>
>> That sounds good. Sorry if we weren't clear at first.
>>
>> Have a good holiday weekend!
>>
>> Sarah & Matt
>>
>>
>> On Fri, Apr 28, 2017 at 4:35 AM, Khushboo Vashi <
>> khushboo.va...@enterprisedb.com> wrote:
>>
>>> Hi Sarah,
>>>
>>> On Thu, Apr 27, 2017 at 7:38 PM, Sarah McAlear 
>>> wrote:
>>>
>>>> Hi Kushboo!
>>>>
>>>> We understand your point, but we believe that relying on 2 independent
>>>> functions to deliver the same formatting can become a problem if the PG
>>>> function changes. Our suggestion is to use a single function in our
>>>> javascript code to do this formatting.
>>>>
>>>> It seems reasonable to me and I am going to use a single javascript
>>> function which will support PB also (as per Dave we should add support till
>>> PB) .
>>>
>>>> If the community believes we can live with this risk, let's move
>>>> forward.
>>>>
>>>> Thanks!
>>>> Sarah & Joao
>>>>
>>>> On Thu, Apr 27, 2017 at 1:50 AM, Khushboo Vashi <
>>>> khushboo.va...@enterprisedb.com> wrote:
>>>>
>>>>> Hi Joao & Sarah,
>>>>>
>>>>> On Wed, Apr 26, 2017 at 8:46 PM, Joao Pedro De Almeida Pereira <
>>>>> jdealmeidapere...@pivotal.io> wrote:
>>>>>
>>>>>> Hi Khushboo!
>>>>>>
>>>>>> Thanks for your reply!
>>>>>>
>>>>>>
>>>>>>> *SQL Files:*
>>>>>>>>
>>>>>>>>- Is there a way to avoid conditionals here?
>>>>>>>>
>>>>>>>>
>>>>>>>>- Maybe we can use the same javascript function to prettify all
>>>>>>>>the sizes
>>>>>>>>
>>>>>>>>
>>>>>>>> In case of collection node (ex: Databases), I have implemented this
>>>>>>>> functionality via putting a formatter for a back-grid column. So, it is
>>>>>>>> applicable for the entire column not for the individual cell. To apply 
>>>>>>>> the
>>>>>>>> javascript function on a single cell for the column (string column), 
>>>>>>>> first
>>>>>>>> we need to identify that cell and then apply the function, I think 
>>>>>>>> this is
>>>>>>>> overhead. Just to avoid conditional sql template I would not prefer 
>>>>>>>> this
>>>>>>>> approach.
>>>>>>>
>>>>>>>
>>>>>> We are not totally sure we understood what you meant on the previous
>>>>>> statement. Are you saying that the conditionals in SQL are used to ensure
>>>>>> that we can apply a javascript function at column level instead of cell
>>>>>> level?
>>>>>>
>>>>>> Correct.
>>>>>
>>>>>> Our concern is that the templates are being made more complex and
>>>>>> inconsistencies are introduced in the code and the UI.
>>>>>>
>>>>>
>>>>> Inconsistencies in the UI can be avoided through making the
>>>>> size_formatter same as pg_size_pretty function which I will implement.
>>>>> I have checked the pg_size_pretty function code and it supports till
>>>>> TB format, so I think we should keep till TB only.
>>>>>
>>>>> In this particular example

Re: [pgadmin-hackers] Pains and thoughts about refactoring the Tree Menu using React

[Joao Pedro De Almeida Pereira]

>
>
> Apart from them, we also need to understand, how the tree traversal will
> work.
> How would you traverse through a particular node, and expand all its
> parent?


 In the PoC that we sent with React Tree email
<https://www.postgresql.org/message-id/CAFS4TJavz7yp2yOiKApV5w3rpaAH1N5ahDdB92UR28pAjK=4...@mail.gmail.com>
we
added a service that as a function called buildURL, that would map into the
GenerateURLAdaptor, and this function will create the URL to retrieve the
child nodes of a specific node. Once we have retrieved those child nodes
from the backend api, we would simply add the new nodes as children in the
tree.

Joao & Matt


On Fri, Apr 28, 2017 at 5:51 AM, Ashesh Vashi  wrote:

> On Sat, Apr 22, 2017 at 12:17 AM, Joao Pedro De Almeida Pereira <
> jdealmeidapere...@pivotal.io> wrote:
>
>> Hi Hackers,
>>
>> After a conversation with Dave we believe that we need to provide more
>> context on our pains and what we propose as a first step for implementing
>> the Tree Menu using React.
>>
>> The pain:
>>
>> Here is a quick description of what we think we understand and where we
>> got stuck while doing our refactor.
>>
>> If you look at the above image, both the NodeJS and CollectionJS classes
>> are templates. Our goal was to extract those from being templates so that
>> we could test them. We started with the generate_url function. The issue we
>> ran into was that the generate_url function is inherited by CollectionJS
>> from NodeJS but then over writes some of the functionality that it
>> inherits. In order to have one function that our react component would
>> delegate to, we have to consolidate these two class methods. This turned
>> out to be a non-trivial refactor due to the lack of tests/documentation.
>>
>> Our proposal:
>>
>>
>> This diagram represents what we believe can be our first approach to
>> updating the tree. We can create a very basic tree that only does tree
>> stuff (Open Node, Close Nodes) and then delegates to other adapters to
>> execute all the business logic functionality (Generate URLs to get the
>> node, Right Click menu, …)
>>
> We will loose the current extendible architecture of by creating fix
> number of adapters, instead - we should generates events like right now,
> let the modules/extensions decide - what to do.
>
> There are many operations are dependents on the tree events.
> e.g.
> - Before open
> When server node is being opened, 'beforeopen' event checks if server is
> connected, or not.
> If not try to connect it, otherwise it does not let it open it
>
> - Added
> Dependent modules libraries are loaded, when a particular type of node is
> added.
> i.e. When the first database node is added in the tree, it loads all the
> schema level, and other javascript modules.
>
> - Selected
> When any of the node is added, it shows properties of the selected, change
> the dashboard context (if necessary).
>
> Current tree generates many events.
> beforeload, beforeappend, added, appended, loaded, init, beforeselect,
> selected, beforefocus, focused, focus, blurred, beforetoggle, beforeopen,
> openned, toggled, etc.
>
> We generate 'pgadmin-browser:tree' events on a pgBrowser.Events objects to
> be used by different modules.
> At the moment, 'pgadmin-browser:tree:selected' is widely used by
> different modules.
> i.e. SQL, dependents, dependencies, properties, Dashboard, etc.
>
> If you want to see the complete list of events generated, you can use the
> following patch.
>
> *diff --git a/web/pgadmin/browser/static/js/datamodel.js
> b/web/pgadmin/browser/static/js/datamodel.js*
> *index 5b1c3a7..575c78d 100644*
> *--- a/web/pgadmin/browser/static/js/datamodel.js*
> *+++ b/web/pgadmin/browser/static/js/datamodel.js*
> *@@ -1149,5 +1149,10 @@ function(_, pgAdmin, $, Backbone) {*
>
> * pgBrowser.Events = _.extend({}, Backbone.Events);*
>
> *+pgBrowser.Events.on(*
> *+  'pgadmin-browser:tree', function() {*
> *+console.log(arguments[0]);*
> *+  });*
> *+*
> * return pgBrowser;*
> * });*
>
> Apart from them, we also need to understand, how the tree traversal will
> work.
> How would you traverse through a particular node, and expand all its
> parent?
>
>
>> Asks:
>>
>>-
>>
>>Are there any more operation that currently are being triggered by
>>the tree? If so we need to add them to the Adapter List.
>>
>> Please see my above comments.
>
>>
>>-
>>
>>Because we lack the context and knowledge of the current tree
>>implem

Re: [pgadmin-hackers][patch] Dependents and Dependencies in GreenPlum

[Joao Pedro De Almeida Pereira]

Thanks for reviewing, Ashesh.

We have updated the patch. The headers are all consistent and we removed
the __init__.py files in directories containing only .sql.

Thanks!
Joao & Matt

On Wed, Apr 26, 2017 at 11:22 AM, Joao Pedro De Almeida Pereira <
jdealmeidapere...@pivotal.io> wrote:

> Hello Ashesh,
>
> Thanks for reviewing the patch.
>
> We added the __init__.py files into templates to convert them into
> packages so that the tests inside of them can be found by the test runner.
>
> Thanks!
> Joao & Sarah
>
> On Wed, Apr 26, 2017 at 1:26 AM, Ashesh Vashi <
> ashesh.va...@enterprisedb.com> wrote:
>
>> On Mon, Apr 24, 2017 at 4:43 PM, Dave Page  wrote:
>>
>>> Ashesh, can you review/commit this please?
>>>
>>> On Fri, Apr 21, 2017 at 8:42 PM, Joao Pedro De Almeida Pereira <
>>> jdealmeidapere...@pivotal.io> wrote:
>>>
>>>> Hi Hackers,
>>>>
>>>> We found out that when you are connected to a GreenPlum database and
>>>> try to get Dependents and Dependencies of an object the application was
>>>> returning a SQL error.
>>>>
>>>> This patch splits the SQL query used to retrieve the Dependents,
>>>> Dependencies, and Roles SQL file into multiple versioned files.
>>>> Add Unit Tests for each file.
>>>> Also added __init__.py files to other test directories to run the tests
>>>> in them.
>>>>
>>> Hi Joao & Sarah,
>>
>> Why do we need to add __init__.py in the template directory?
>> I didn't understand the purpose of the adding __init__.py files in the
>> template directories.
>>
>> NOTE: The headers in those files are not consistent with the other
>> project files.
>>
>> --
>>
>> Thanks & Regards,
>>
>> Ashesh Vashi
>> EnterpriseDB INDIA: Enterprise PostgreSQL Company
>> <http://www.enterprisedb.com/>
>>
>>
>> *http://www.linkedin.com/in/asheshvashi*
>> <http://www.linkedin.com/in/asheshvashi>
>>
>>> Add ORDER BY into Copy Selection Feature test to ensure the results are
>>>> retrieved always in the same order
>>>> Renamed the Scenario of the xss_checks_pgadmin_debugger_test and skip
>>>> it for versions less than 9.1
>>>>
>>>> Thanks
>>>>
>>>> Joao & Sarah
>>>>
>>>>
>>>> --
>>>> Sent via pgadmin-hackers mailing list (pgadmin-hackers@postgresql.org)
>>>> To make changes to your subscription:
>>>> http://www.postgresql.org/mailpref/pgadmin-hackers
>>>>
>>>>
>>>
>>>
>>> --
>>> Dave Page
>>> Blog: http://pgsnake.blogspot.com
>>> Twitter: @pgsnake
>>>
>>> EnterpriseDB UK: http://www.enterprisedb.com
>>> The Enterprise PostgreSQL Company
>>>
>>
>>
>


0001-Fix-error-while-checking-Dependents-and-Dependencies.patch
Description: Binary data

-- 
Sent via pgadmin-hackers mailing list (pgadmin-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgadmin-hackers

Re: [pgadmin-hackers] Issue with SlickGrid

[Joao Pedro De Almeida Pereira]

Hello Murtuza,
Thanks for the explanation. Based on what you said it looks like a bug in
the library, have you guys considered sending a PR to it?

Thanks

On Thu, Apr 27, 2017, 2:46 AM Murtuza Zabuawala <
murtuza.zabuaw...@enterprisedb.com> wrote:

> +++
> Reference:
> https://www.postgresql.org/message-id/CAKKotZRjqbKAZev81Zk78nikDVXqLKEDV5r%2BsW8Me31Gpzrm_A%40mail.gmail.com
>
> --
> Regards,
> Murtuza Zabuawala
> EnterpriseDB: http://www.enterprisedb.com
> The Enterprise PostgreSQL Company
>
> On Thu, Apr 27, 2017 at 12:09 PM, Murtuza Zabuawala <
> murtuza.zabuaw...@enterprisedb.com> wrote:
>
>> Hello Joao,
>>
>> Yes, We made some changes in SlickGrid library when we integrated it into
>> Query tool.
>>
>> *Issue:* Last row from the query result set was not displaying correctly
>> in query tool when we have scrollbar in grid.
>>
>> The row hight/width pixel size calculations is done inside SlickGrid
>> javascript code, Though we tried solve it through CSS but we had no luck,
>> so we had no other choice but to do it in library it self.
>>
>> The changes were,
>> 1) "getDataLengthIncludingAddNew()" function (slick.grid.js) to add two
>> new rows instead of one when user add values into row (one row is dummy &
>> not visible to user so that it displays last row correctly)
>> 2) Other change was done into "appendRowHtml()" function to calculating
>> the correct number of rows in SlickGrid result as we have added our own
>> custom row as mentioned earlier.
>> 3) Abbreviated long CSS classes as mentioed in README file.
>>
>> Apologies we missed to update this change in README.
>>
>>
>> --
>> Regards,
>> Murtuza Zabuawala
>> EnterpriseDB: http://www.enterprisedb.com
>> The Enterprise PostgreSQL Company
>>
>> On Thu, Apr 27, 2017 at 2:23 AM, Joao Pedro De Almeida Pereira <
>> jdealmeidapere...@pivotal.io> wrote:
>>
>>> Hello Hackers,
>>>
>>> While doing some changes to the Query Results we found out that there
>>> was a issue with Slick grid.
>>>
>>> The issue that we found was with the CellSelectModel, behaved
>>> differently when pressing Ctrl and Command(Mac). We created a PR
>>> <https://github.com/6pac/SlickGrid/pull/100> with the change to changes
>>> the behavior of the plugin.
>>>
>>> When this PR is applied to the SlickGrid library we need to apply it to
>>> the current version of SlickGrid that we have vendorized.
>>> According to the libraries.txt file we are in version 2.2.4 of the
>>> library but a diff between our code and the libraries version 2.2.4 shows
>>> differences in the code.
>>>
>>> Did we do any change to SlickGrid library that is vendorized? Or is just
>>> the information in libraries.txt that is incorrect?
>>> Does anyone know any problem if we bump the version of SlickGrid to the
>>> newer version after the PR is applied?
>>>
>>> Thanks
>>> Joao
>>>
>>
>>
>

[pgadmin-hackers] Issue with SlickGrid

[Joao Pedro De Almeida Pereira]

Hello Hackers,

While doing some changes to the Query Results we found out that there was a
issue with Slick grid.

The issue that we found was with the CellSelectModel, behaved differently
when pressing Ctrl and Command(Mac). We created a PR
 with the change to changes the
behavior of the plugin.

When this PR is applied to the SlickGrid library we need to apply it to the
current version of SlickGrid that we have vendorized.
According to the libraries.txt file we are in version 2.2.4 of the library
but a diff between our code and the libraries version 2.2.4 shows
differences in the code.

Did we do any change to SlickGrid library that is vendorized? Or is just
the information in libraries.txt that is incorrect?
Does anyone know any problem if we bump the version of SlickGrid to the
newer version after the PR is applied?

Thanks
Joao

Re: [pgadmin-hackers][patch] Dependents and Dependencies in GreenPlum

[Joao Pedro De Almeida Pereira]

Hello Ashesh,

Thanks for reviewing the patch.

We added the __init__.py files into templates to convert them into packages
so that the tests inside of them can be found by the test runner.

Thanks!
Joao & Sarah

On Wed, Apr 26, 2017 at 1:26 AM, Ashesh Vashi  wrote:

> On Mon, Apr 24, 2017 at 4:43 PM, Dave Page  wrote:
>
>> Ashesh, can you review/commit this please?
>>
>> On Fri, Apr 21, 2017 at 8:42 PM, Joao Pedro De Almeida Pereira <
>> jdealmeidapere...@pivotal.io> wrote:
>>
>>> Hi Hackers,
>>>
>>> We found out that when you are connected to a GreenPlum database and try
>>> to get Dependents and Dependencies of an object the application was
>>> returning a SQL error.
>>>
>>> This patch splits the SQL query used to retrieve the Dependents,
>>> Dependencies, and Roles SQL file into multiple versioned files.
>>> Add Unit Tests for each file.
>>> Also added __init__.py files to other test directories to run the tests
>>> in them.
>>>
>> Hi Joao & Sarah,
>
> Why do we need to add __init__.py in the template directory?
> I didn't understand the purpose of the adding __init__.py files in the
> template directories.
>
> NOTE: The headers in those files are not consistent with the other project
> files.
>
> --
>
> Thanks & Regards,
>
> Ashesh Vashi
> EnterpriseDB INDIA: Enterprise PostgreSQL Company
> <http://www.enterprisedb.com/>
>
>
> *http://www.linkedin.com/in/asheshvashi*
> <http://www.linkedin.com/in/asheshvashi>
>
>> Add ORDER BY into Copy Selection Feature test to ensure the results are
>>> retrieved always in the same order
>>> Renamed the Scenario of the xss_checks_pgadmin_debugger_test and skip
>>> it for versions less than 9.1
>>>
>>> Thanks
>>>
>>> Joao & Sarah
>>>
>>>
>>> --
>>> Sent via pgadmin-hackers mailing list (pgadmin-hackers@postgresql.org)
>>> To make changes to your subscription:
>>> http://www.postgresql.org/mailpref/pgadmin-hackers
>>>
>>>
>>
>>
>> --
>> Dave Page
>> Blog: http://pgsnake.blogspot.com
>> Twitter: @pgsnake
>>
>> EnterpriseDB UK: http://www.enterprisedb.com
>> The Enterprise PostgreSQL Company
>>
>
>

Re: [pgadmin-hackers] [pgAdmin4][Patch]: Fixed RM #2315 : Sorting by size is broken

[Joao Pedro De Almeida Pereira]

Hi Khushboo!

Thanks for your reply!


> *SQL Files:*
>>
>>- Is there a way to avoid conditionals here?
>>
>>
>>- Maybe we can use the same javascript function to prettify all the
>>sizes
>>
>>
>> In case of collection node (ex: Databases), I have implemented this
>> functionality via putting a formatter for a back-grid column. So, it is
>> applicable for the entire column not for the individual cell. To apply the
>> javascript function on a single cell for the column (string column), first
>> we need to identify that cell and then apply the function, I think this is
>> overhead. Just to avoid conditional sql template I would not prefer this
>> approach.
>
>
We are not totally sure we understood what you meant on the previous
statement. Are you saying that the conditionals in SQL are used to ensure
that we can apply a javascript function at column level instead of cell
level?

Our concern is that the templates are being made more complex and
inconsistencies are introduced in the code and the UI. In this particular
example, we are allowing the backend to respond sometimes with prettified
data and sometimes without it, so at UI level we will have inconsistencies
between screens or more complex Javascript code to support sometimes
prettifying and sometimes not prettify the same fields.

What we were thinking was to maybe not specify on the SQL level and have
the same format for "Size" everywhere in the UI.

Thanks
Joao & Sarah

On Tue, Apr 25, 2017 at 11:48 PM, Khushboo Vashi <
khushboo.va...@enterprisedb.com> wrote:

> Hi Joao & Sarah,
>
> Thanks for reviewing the patch.
>
> On Tue, Apr 25, 2017 at 8:34 PM, Joao Pedro De Almeida Pereira <
> jdealmeidapere...@pivotal.io> wrote:
>
>> Hello Khushboo,
>>
>> We reviewed the this patch and have some suggestions:
>>
>> *Python:*
>> ​
>> The functionality for adding the "can_prettify" is repeated in multiple
>> places. Maybe this could be extracted into a function.
>>
>> When I have implemented this, my first thought is exactly same as you
> suggested but  while looking at the code I felt its not a good idea to have
> a function. In case of a function, we need to pass the whole result-set as
> well as the list of fields which we need to be prettified. So, only for 2
> lines, I didn't find any reason to make a function.
>
>> *Javascript:*
>> ​
>>
>>- The class Backgrid.SizeFormatter doesn't seem to have any tests.
>>
>>
>> Sure, will do.
>
>>
>>- The function pg_size_pretty displays bytes and Kilobytes
>>differently.
>>- Is it possible to add PB as well?
>>
>> Will check and add PB.
>
>>
>>-
>>- The function is a little bit hard to read, is it possible to
>>refactor using private functions like:
>>
>> Will make it more readable.
>
>> fromRaw: function (rawData, model) {
>>var unitIdx = findDataUnitIndex(rawData);
>>if (unitIdx == 0) {
>>   return rawData + ' ' + this.dataUnits[i];
>>}
>>return formatOutput(rawData, unitIdx);
>> },
>>
>> ​
>>
>>
>>- In statistics.js:326 we believe it would make the code more
>>readable if we change the variable "c" to "rawColumn" and "col" to 
>> "column".
>>
>>
>> I will change the variable name from  "c" to  "rawColumn" but I think
> "col" is appropriate as we already have columns variable and anyone can
> confuse while reading the code (for columns and column).
>
>>
>> *SQL Files:*
>> ​
>>
>>- Is there a way to avoid conditionals here?
>>- Maybe we can use the same javascript function to prettify all the
>>sizes
>>
>>
>> In case of collection node (ex: Databases), I have implemented this
> functionality via putting a formatter for a back-grid column. So, it is
> applicable for the entire column not for the individual cell. To apply the
> javascript function on a single cell for the column (string column), first
> we need to identify that cell and then apply the function, I think this is
> overhead. Just to avoid conditional sql template I would not prefer this
> approach.
>
>>
>> Visually we saw a difference between "Databases" statistics and a
>> specific database statistics. In "Databases" statistics the "Size" is "7.4
>> MB" but when you are in the specific database the "Size" is "7420 kB".
>> Is this the intended behavior?
>>

Re: [pgadmin-hackers] [pgAdmin4][Patch]: Fixed RM #2315 : Sorting by size is broken

[Joao Pedro De Almeida Pereira]

Hello Khushboo,

We reviewed the this patch and have some suggestions:

*Python:*
​
The functionality for adding the "can_prettify" is repeated in multiple
places. Maybe this could be extracted into a function.

*Javascript:*
​

   - The class Backgrid.SizeFormatter doesn't seem to have any tests.



   - The function pg_size_pretty displays bytes and Kilobytes differently.
   - Is it possible to add PB as well?
   - The function is a little bit hard to read, is it possible to refactor
   using private functions like:

fromRaw: function (rawData, model) {
   var unitIdx = findDataUnitIndex(rawData);
   if (unitIdx == 0) {
  return rawData + ' ' + this.dataUnits[i];
   }
   return formatOutput(rawData, unitIdx);
},

​


   - In statistics.js:326 we believe it would make the code more readable
   if we change the variable "c" to "rawColumn" and "col" to "column".



*SQL Files:*
​

   - Is there a way to avoid conditionals here?
   - Maybe we can use the same javascript function to prettify all the sizes



Visually we saw a difference between "Databases" statistics and a specific
database statistics. In "Databases" statistics the "Size" is "7.4 MB" but
when you are in the specific database the "Size" is "7420 kB".
Is this the intended behavior?



Thanks
Joao & Sarah

On Tue, Apr 25, 2017 at 7:58 AM, Dave Page  wrote:

> Ashesh, can you review/commit this please?
>
> Thanks.
>
> On Tue, Apr 25, 2017 at 10:18 AM, Khushboo Vashi <
> khushboo.va...@enterprisedb.com> wrote:
>
>> Hi,
>>
>> Fixed RM #2315 : Sorting by size is broken.
>>
>> Removed the pg_size_pretty function from query for the collection and
>> introduced the client side function to convert size into human readable
>> format. So, the sorting issue is fixed as the algorithm will get the actual
>> value of size instead of formatted value.
>>
>>
>> Thanks,
>> Khushboo
>>
>>
>>
>>
>> --
>> Sent via pgadmin-hackers mailing list (pgadmin-hackers@postgresql.org)
>> To make changes to your subscription:
>> http://www.postgresql.org/mailpref/pgadmin-hackers
>>
>>
>
>
> --
> Dave Page
> Blog: http://pgsnake.blogspot.com
> Twitter: @pgsnake
>
> EnterpriseDB UK: http://www.enterprisedb.com
> The Enterprise PostgreSQL Company
>

[pgadmin-hackers][patch] Dependents and Dependencies in GreenPlum

[Joao Pedro De Almeida Pereira]

Hi Hackers,

We found out that when you are connected to a GreenPlum database and try to
get Dependents and Dependencies of an object the application was returning
a SQL error.

This patch splits the SQL query used to retrieve the Dependents,
Dependencies, and Roles SQL file into multiple versioned files.
Add Unit Tests for each file.
Also added __init__.py files to other test directories to run the tests in
them.
Add ORDER BY into Copy Selection Feature test to ensure the results are
retrieved always in the same order
Renamed the Scenario of the xss_checks_pgadmin_debugger_test and skip it
for versions less than 9.1

Thanks

Joao & Sarah


0001-Fix-error-while-checking-Dependents-and-Dependencies.patch
Description: Binary data

-- 
Sent via pgadmin-hackers mailing list (pgadmin-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgadmin-hackers

Re: [pgadmin-hackers][patch] Update chrome driver to support chrome version 58 in tests

[Joao Pedro De Almeida Pereira]

Hi Hackers,

We recreated the patch and should work now.

The problem looks like an issue with pip. I just opened an issue in their
github <https://github.com/pypa/pip/issues/4453>.

Thanks
Joao & Oliver

On Fri, Apr 21, 2017 at 5:20 AM, Dave Page  wrote:

> Hi,
>
> Unfortunately I've had to revert this as the Jenkins CI builds started
> failing on all branches. It appears that the --install-option for
> chromerdriver_installer is also passed to pyperclip, which then barfs on it:
>
> /var/lib/jenkins/workspace/pgadmin4-master-python27/pgadmin-venv/lib/python2.7/site-packages/pip/req/req_file.py:150:
>  UserWarning: Disabling all use of wheels due to the use of --build-options / 
> --global-options / --install-options.
>   cmdoptions.check_install_build_global(options, opts)
> Collecting chromedriver_installer==0.0.6 (from -r 
> web/regression/requirements.txt (line 1))
>   Using cached chromedriver_installer-0.0.6.tar.gz
> Collecting pyperclip~=1.5.27 (from -r web/regression/requirements.txt (line 
> 2))
>   Using cached pyperclip-1.5.27.zip
> Collecting selenium==3.3.1 (from -r web/regression/requirements.txt (line 3))
>   Using cached selenium-3.3.1.tar.gz
> Collecting testscenarios==0.5.0 (from -r web/regression/requirements.txt 
> (line 4))
>   Using cached testscenarios-0.5.0.tar.gz
> Collecting testtools==2.0.0 (from -r web/regression/requirements.txt (line 5))
>   Using cached testtools-2.0.0.tar.gz
> Requirement already satisfied: traceback2==1.4.0 in 
> ./pgadmin-venv/lib/python2.7/site-packages (from -r 
> web/regression/requirements.txt (line 6))
> Requirement already satisfied: unittest2==1.1.0 in 
> ./pgadmin-venv/lib/python2.7/site-packages (from -r 
> web/regression/requirements.txt (line 7))
> Requirement already satisfied: pbr>=0.11 in 
> ./pgadmin-venv/lib/python2.7/site-packages (from testscenarios==0.5.0->-r 
> web/regression/requirements.txt (line 4))
> Requirement already satisfied: extras in 
> ./pgadmin-venv/lib/python2.7/site-packages (from testtools==2.0.0->-r 
> web/regression/requirements.txt (line 5))
> Requirement already satisfied: fixtures>=1.3.0 in 
> ./pgadmin-venv/lib/python2.7/site-packages (from testtools==2.0.0->-r 
> web/regression/requirements.txt (line 5))
> Requirement already satisfied: pyrsistent in 
> ./pgadmin-venv/lib/python2.7/site-packages (from testtools==2.0.0->-r 
> web/regression/requirements.txt (line 5))
> Requirement already satisfied: python-mimeparse in 
> ./pgadmin-venv/lib/python2.7/site-packages (from testtools==2.0.0->-r 
> web/regression/requirements.txt (line 5))
> Requirement already satisfied: linecache2 in 
> ./pgadmin-venv/lib/python2.7/site-packages (from traceback2==1.4.0->-r 
> web/regression/requirements.txt (line 6))
> Requirement already satisfied: argparse in 
> ./pgadmin-venv/lib/python2.7/site-packages (from unittest2==1.1.0->-r 
> web/regression/requirements.txt (line 7))
> Requirement already satisfied: six>=1.4 in 
> ./pgadmin-venv/lib/python2.7/site-packages (from unittest2==1.1.0->-r 
> web/regression/requirements.txt (line 7))
> Skipping bdist_wheel for chromedriver-installer, due to binaries being 
> disabled for it.
> Skipping bdist_wheel for pyperclip, due to binaries being disabled for it.
> Skipping bdist_wheel for selenium, due to binaries being disabled for it.
> Skipping bdist_wheel for testscenarios, due to binaries being disabled for it.
> Skipping bdist_wheel for testtools, due to binaries being disabled for it.
> Installing collected packages: chromedriver-installer, pyperclip, selenium, 
> testtools, testscenarios
>   Running setup.py install for chromedriver-installer: started
> Running setup.py install for chromedriver-installer: finished with status 
> 'done'
>   Running setup.py install for pyperclip: started
> Running setup.py install for pyperclip: finished with status 'error'
> Complete output from command 
> /var/lib/jenkins/workspace/pgadmin4-master-python27/pgadmin-venv/bin/python 
> -u -c "import setuptools, 
> tokenize;__file__='/tmp/pip-build-YzPhsw/pyperclip/setup.py';f=getattr(tokenize,
>  'open', open)(__file__);code=f.read().replace('\r\n', 
> '\n');f.close();exec(compile(code, __file__, 'exec'))" install --record 
> /tmp/pip-Sy2sJJ-record/install-record.txt --single-version-externally-managed 
> --compile --install-headers 
> /var/lib/jenkins/workspace/pgadmin4-master-python27/pgadmin-venv/include/site/python2.7/pyperclip
>  --chromedriver-version=2.29:
> usage: -c [global_opts] cmd1 [cmd1_opts] [cmd2 [cmd2_opts] ...]
>or: -c --help [cmd1 cmd2 ...]
>or: -c --help-commands
>or: -c cmd --he

Re: [pgadmin-hackers] Pains and thoughts about refactoring the Tree Menu using React

[Joao Pedro De Almeida Pereira]

Hi Hackers,

The attachment was missing in the previous email, so there you go.

Note:
 We had to remove the extension of the file due to security reasons, just
add a .js

Thanks
Joao & Oliver

On Fri, Apr 21, 2017 at 2:47 PM, Joao Pedro De Almeida Pereira <
jdealmeidapere...@pivotal.io> wrote:

> Hi Hackers,
>
> After a conversation with Dave we believe that we need to provide more
> context on our pains and what we propose as a first step for implementing
> the Tree Menu using React.
>
> The pain:
>
> Here is a quick description of what we think we understand and where we
> got stuck while doing our refactor.
>
> If you look at the above image, both the NodeJS and CollectionJS classes
> are templates. Our goal was to extract those from being templates so that
> we could test them. We started with the generate_url function. The issue we
> ran into was that the generate_url function is inherited by CollectionJS
> from NodeJS but then over writes some of the functionality that it
> inherits. In order to have one function that our react component would
> delegate to, we have to consolidate these two class methods. This turned
> out to be a non-trivial refactor due to the lack of tests/documentation.
>
> Our proposal:
>
>
> This diagram represents what we believe can be our first approach to
> updating the tree. We can create a very basic tree that only does tree
> stuff (Open Node, Close Nodes) and then delegates to other adapters to
> execute all the business logic functionality (Generate URLs to get the
> node, Right Click menu, …)
>
> Asks:
>
>-
>
>Are there any more operation that currently are being triggered by the
>tree? If so we need to add them to the Adapter List.
>-
>
>Because we lack the context and knowledge of the current tree
>implementation, we need your help to extract these methods from the places
>they are currently in. We believe that a good place to start would be
>writing tests for and implementing the generation of URLs used to fetch the
>child nodes.
>
> Attached you can find an example of the JavaScript tests that we have been
> writing that can be applied to this extracted method.
>
>
> Thanks
>
> Rob, Oliver & Joao
>
>


node_url_generator_spec
Description: Binary data

-- 
Sent via pgadmin-hackers mailing list (pgadmin-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgadmin-hackers

[pgadmin-hackers] Pains and thoughts about refactoring the Tree Menu using React

[Joao Pedro De Almeida Pereira]

Hi Hackers,

After a conversation with Dave we believe that we need to provide more
context on our pains and what we propose as a first step for implementing
the Tree Menu using React.

The pain:

Here is a quick description of what we think we understand and where we got
stuck while doing our refactor.

If you look at the above image, both the NodeJS and CollectionJS classes
are templates. Our goal was to extract those from being templates so that
we could test them. We started with the generate_url function. The issue we
ran into was that the generate_url function is inherited by CollectionJS
from NodeJS but then over writes some of the functionality that it
inherits. In order to have one function that our react component would
delegate to, we have to consolidate these two class methods. This turned
out to be a non-trivial refactor due to the lack of tests/documentation.

Our proposal:


This diagram represents what we believe can be our first approach to
updating the tree. We can create a very basic tree that only does tree
stuff (Open Node, Close Nodes) and then delegates to other adapters to
execute all the business logic functionality (Generate URLs to get the
node, Right Click menu, …)

Asks:

   -

   Are there any more operation that currently are being triggered by the
   tree? If so we need to add them to the Adapter List.
   -

   Because we lack the context and knowledge of the current tree
   implementation, we need your help to extract these methods from the places
   they are currently in. We believe that a good place to start would be
   writing tests for and implementing the generation of URLs used to fetch the
   child nodes.

Attached you can find an example of the JavaScript tests that we have been
writing that can be applied to this extracted method.


Thanks

Rob, Oliver & Joao

Re: [pgadmin-hackers][patch] Move to Alembic migration system

[Joao Pedro De Almeida Pereira]

Hello Hackers,

We review the patch, just noticed a spelling issue so we regenerated the
patch.

Thanks
Joao & Oliver

On Fri, Apr 21, 2017 at 1:21 AM, Ashesh Vashi  wrote:

> Hi Joao & Oliver,
>
> On Fri, Apr 21, 2017 at 3:39 AM, Joao Pedro De Almeida Pereira <
> jdealmeidapere...@pivotal.io> wrote:
>
>> Hello Hackers,
>>
>> @Ashesh thanks for the feedback
>>
>> Here is the reviewed patch with the suggestions of Ashesh.
>>
>> Disclaimer: We added a new patch file with the changes
>>
>
> I have made some more changes to the patch.
> - 'with app.app_context(..)' statement was not required in the
> 'web/pgadmin/__init__.py' as we're already doing that in the do_upgrade
> function.
> - We also need to create other directories (i.e. sessions, storage,
> directory containing the log-file) during the setup/running the application
> (if not exists).
> - Added proper check in the pgAdmin4.wsgi file (if configuration file
> exists, or not)
>
> Please review it.
>
> -- Thanks, Ashesh
>
>


0001-Switch-to-Alembic-and-Flask-migration-db-migration-s.patch
Description: Binary data

-- 
Sent via pgadmin-hackers mailing list (pgadmin-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgadmin-hackers

Re: [pgadmin-hackers][patch] Move to Alembic migration system

[Joao Pedro De Almeida Pereira]

Hello Hackers,

@Ashesh thanks for the feedback

Here is the reviewed patch with the suggestions of Ashesh.

Disclaimer: We added a new patch file with the changes


Thanks
Joao & Oliver

On Thu, Apr 20, 2017 at 10:56 AM, Ashesh Vashi <
ashesh.va...@enterprisedb.com> wrote:

> On Thu, Apr 20, 2017 at 8:15 PM, Joao Pedro De Almeida Pereira <
> jdealmeidapere...@pivotal.io> wrote:
>
>> Hello Ashesh,
>>
>> Did you had time to review this patch?
>>
> Yes - I did.
>
> Please my review comments:
> Everytime I start pgAdmin 4, I am getting now the following message:
> "pgAdmin 4 - Application Initialisation"
>
> Can we avoid that message?
> We are not providing any information during upgrade process.
> i.e. older version was so, and so, it will be upgraded to so, and so.
>
> It helps sometimes to while debugging the application.
>
> Rest looks fine to me.
>
> -- Thanks, Ashesh
>
>>
>> Thanks
>> Joao & Oliver
>>
>> On Wed, Apr 12, 2017 at 9:52 AM, Sarah McAlear 
>> wrote:
>>
>>> Great, thank you so much!
>>>
>>> On Wed, Apr 12, 2017 at 9:41 AM, Ashesh Vashi <
>>> ashesh.va...@enterprisedb.com> wrote:
>>>
>>>>
>>>> On Wed, Apr 12, 2017 at 7:07 PM, Sarah McAlear 
>>>> wrote:
>>>>
>>>>> Hi Hackers!
>>>>>
>>>> Hi Sarah,
>>>>
>>>>>
>>>>> Is there an update on this?
>>>>>
>>>> We will look in to it end of this week.
>>>> I was not rushing to it, because - Dave was preparing for the 1.4
>>>> release.
>>>>
>>>> -- Thanks, Ashesh
>>>>
>>>>>
>>>>> Thanks,
>>>>> Sarah & Joao
>>>>>
>>>>> On Fri, Apr 7, 2017 at 10:27 AM, Sarah McAlear 
>>>>> wrote:
>>>>>
>>>>>> Hi Ashesh!
>>>>>>
>>>>>> Good catch. Looks like there was an override of the input function
>>>>>> that didn't get moved to the new file, causing the input with the @ to
>>>>>> fail. We also added headers to the files that were missing them. This new
>>>>>> patch should work.
>>>>>>
>>>>>> Thanks!
>>>>>> Joao & Sarah
>>>>>>
>>>>>>
>>>>>>
>>>>>> On Mon, Apr 3, 2017 at 8:41 AM, Ashesh Vashi <
>>>>>> ashesh.va...@enterprisedb.com> wrote:
>>>>>>
>>>>>>> On Mon, Apr 3, 2017 at 12:09 PM, Ashesh Vashi <
>>>>>>> ashesh.va...@enterprisedb.com> wrote:
>>>>>>>
>>>>>>>> Hi Jaoao, Sarah,
>>>>>>>>
>>>>>>>> I've tried to run on fresh machine, it failed with the below error:
>>>>>>>>
>>>>>>> And - I have noticed - the headers are missing in new files.
>>>>>>>
>>>>>>> --
>>>>>>>
>>>>>>> Thanks & Regards,
>>>>>>>
>>>>>>> Ashesh Vashi
>>>>>>> EnterpriseDB INDIA: Enterprise PostgreSQL Company
>>>>>>> <http://www.enterprisedb.com/>
>>>>>>>
>>>>>>>
>>>>>>> *http://www.linkedin.com/in/asheshvashi*
>>>>>>> <http://www.linkedin.com/in/asheshvashi>
>>>>>>>
>>>>>>>>
>>>>>>>> *$ python setup.py*
>>>>>>>> *NOTE: Configuring authentication for SERVER mode.*
>>>>>>>>
>>>>>>>> *Enter the email address and password to use for the initial
>>>>>>>> pgAdmin user account:*
>>>>>>>>
>>>>>>>> *Email address: ashesh.va...@enterprisedb.com
>>>>>>>> *
>>>>>>>> *Traceback (most recent call last):*
>>>>>>>> *  File "setup.py", line 52, in *
>>>>>>>> *db_upgrade(app)*
>>>>>>>> *  File
>>>>>>>> "/Users/asheshvashi/Developments/Projects/pgAdmin4/web/pgadmin/setup/db_upgrade.py",
>>>>>>>> line 25, in db_upgrade*
>>>>>>>> *flask_migrate.upgrade(migration_folder)*
>>>>>>>> *  File
>>>>>>>> &qu

[pgadmin-hackers][patch] Update chrome driver to support chrome version 58 in tests

[Joao Pedro De Almeida Pereira]

Hi Hackers,

With the update to version 58 of chrome the version of the chrome driver of
Selenium need to be updated to version 2.29.

This patch does that change.

Note:
If you had previously installed the old driver you need to uninstall and
install again it.

$ pip uninstall chromedriver_installer
$ pip install -r regression/requirements.txt

​

Thanks
Joao & Oliver


0001-Change-chrome-driver-version-to-2.29-to-make-it-work.patch
Description: Binary data

-- 
Sent via pgadmin-hackers mailing list (pgadmin-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgadmin-hackers

Re: [pgadmin-hackers][patch] Move to Alembic migration system

[Joao Pedro De Almeida Pereira]

online()*
>>>>>> *  File
>>>>>> "/Users/asheshvashi/Developments/Projects/pgAdmin4/web/pgadmin/setup/../../migrations/env.py",
>>>>>> line 78, in run_migrations_online*
>>>>>> *context.run_migrations()*
>>>>>> *  File "", line 8, in run_migrations*
>>>>>> *  File
>>>>>> "/Users/asheshvashi/Developments/WorkPlace/pgAdmin4/lib/python2.7/site-packages/alembic/runtime/environment.py",
>>>>>> line 817, in run_migrations*
>>>>>> *self.get_context().run_migrations(**kw)*
>>>>>> *  File
>>>>>> "/Users/asheshvashi/Developments/WorkPlace/pgAdmin4/lib/python2.7/site-packages/alembic/runtime/migration.py",
>>>>>> line 323, in run_migrations*
>>>>>> *step.migration_fn(**kw)*
>>>>>> *  File
>>>>>> "/Users/asheshvashi/Developments/Projects/pgAdmin4/web/migrations/versions/fdc58d9bd449_.py",
>>>>>> line 84, in upgrade*
>>>>>> *email, password = user_info()*
>>>>>> *  File
>>>>>> "/Users/asheshvashi/Developments/Projects/pgAdmin4/web/pgadmin/setup/user_info.py",
>>>>>> line 50, in user_info*
>>>>>> *email = input("Email address: ")*
>>>>>> *  File "", line 1*
>>>>>> *ashesh.va...@enterprisedb.com *
>>>>>> *        ^*
>>>>>> *SyntaxError: invalid syntax*
>>>>>>
>>>>>>
>>>>>> --
>>>>>>
>>>>>> Thanks & Regards,
>>>>>>
>>>>>> Ashesh Vashi
>>>>>> EnterpriseDB INDIA: Enterprise PostgreSQL Company
>>>>>> <http://www.enterprisedb.com>
>>>>>>
>>>>>>
>>>>>> *http://www.linkedin.com/in/asheshvashi*
>>>>>> <http://www.linkedin.com/in/asheshvashi>
>>>>>>
>>>>>> On Fri, Mar 31, 2017 at 8:17 PM, Murtuza Zabuawala <
>>>>>> murtuza.zabuaw...@enterprisedb.com> wrote:
>>>>>>
>>>>>>> Hi,
>>>>>>>
>>>>>>> PFA minor add-on patch for README.
>>>>>>>
>>>>>>> --
>>>>>>> Regards,
>>>>>>> Murtuza Zabuawala
>>>>>>> EnterpriseDB: http://www.enterprisedb.com
>>>>>>> The Enterprise PostgreSQL Company
>>>>>>>
>>>>>>> On Fri, Mar 31, 2017 at 8:04 PM, Murtuza Zabuawala <
>>>>>>> murtuza.zabuaw...@enterprisedb.com> wrote:
>>>>>>>
>>>>>>>> Hi Ashesh,
>>>>>>>>
>>>>>>>> Patch looks good to me.
>>>>>>>>
>>>>>>>> --
>>>>>>>> Regards,
>>>>>>>> Murtuza Zabuawala
>>>>>>>> EnterpriseDB: http://www.enterprisedb.com
>>>>>>>> The Enterprise PostgreSQL Company
>>>>>>>>
>>>>>>>> On Fri, Mar 31, 2017 at 1:10 PM, Ashesh Vashi <
>>>>>>>> ashesh.va...@enterprisedb.com> wrote:
>>>>>>>>
>>>>>>>>> Hi Joao & Sarah,
>>>>>>>>>
>>>>>>>>> I have asked Murtuza to review the patch today.
>>>>>>>>> He will update me by EOD.
>>>>>>>>>
>>>>>>>>> If all goes well, I will commit the patch.
>>>>>>>>>
>>>>>>>>> --
>>>>>>>>>
>>>>>>>>> Thanks & Regards,
>>>>>>>>>
>>>>>>>>> Ashesh Vashi
>>>>>>>>> EnterpriseDB INDIA: Enterprise PostgreSQL Company
>>>>>>>>> <http://www.enterprisedb.com>
>>>>>>>>>
>>>>>>>>>
>>>>>>>>> *http://www.linkedin.com/in/asheshvashi*
>>>>>>>>> <http://www.linkedin.com/in/asheshvashi>
>>>>>>>>>
>>>>>>>>> On Thu, Mar 30, 2017 at 8:36 PM, Joao Pedro De Almeida Pereira <
>>>>>>>>> jdealmeidapere...@pivotal.io> wrote:
>>>>>>>>>
>>>>>>>>>> Hello Dave and Ashesh,
>>>>>>>>>>
>>>>>>>>>> Do you still need us to provide more information about this patch
>>>>>>>>>> or is it ready to be merged?
>>>>>>>>>>
>>>>>>>>>> Thanks
>>>>>>>>>> Joao
>>>>>>>>>>
>>>>>>>>>> On Thu, Mar 23, 2017 at 12:00 PM, Joao Pedro De Almeida Pereira <
>>>>>>>>>> jdealmeidapere...@pivotal.io> wrote:
>>>>>>>>>>
>>>>>>>>>>> Hello Hackers,
>>>>>>>>>>>
>>>>>>>>>>> We found out a issue using Python 3 related to importing modules
>>>>>>>>>>> that we corrected in the patch that is now attached.
>>>>>>>>>>>
>>>>>>>>>>> Also we would like to know the status of this.
>>>>>>>>>>>
>>>>>>>>>>> Thanks
>>>>>>>>>>> Joao & Sarah
>>>>>>>>>>>
>>>>>>>>>>> On Fri, Mar 17, 2017 at 10:32 AM, Sarah McAlear <
>>>>>>>>>>> smcal...@pivotal.io> wrote:
>>>>>>>>>>>
>>>>>>>>>>>> Hi!
>>>>>>>>>>>>
>>>>>>>>>>>> We realized that this change was causing the tests to fail
>>>>>>>>>>>> because the folder for the sqlite databases was not being created. 
>>>>>>>>>>>> We also
>>>>>>>>>>>> updated the files to contain the missing headers.
>>>>>>>>>>>>
>>>>>>>>>>>> Thanks!
>>>>>>>>>>>> Joao & Sarah
>>>>>>>>>>>>
>>>>>>>>>>>>
>>>>>>>>>>>>
>>>>>>>>>>>> On Thu, Mar 16, 2017 at 9:31 AM, Dave Page 
>>>>>>>>>>>> wrote:
>>>>>>>>>>>>
>>>>>>>>>>>>> Ashesh, can you review/commit this please? One thing I notice
>>>>>>>>>>>>> on a
>>>>>>>>>>>>> quick look through is that the file headers are missing
>>>>>>>>>>>>> everywhere.
>>>>>>>>>>>>> They should be present in all source files, except where they
>>>>>>>>>>>>> would
>>>>>>>>>>>>> bloat the data transfer from client to server.
>>>>>>>>>>>>>
>>>>>>>>>>>>> On Wed, Mar 15, 2017 at 8:09 PM, Sarah McAlear <
>>>>>>>>>>>>> smcal...@pivotal.io> wrote:
>>>>>>>>>>>>> > Hi Hackers!
>>>>>>>>>>>>> >
>>>>>>>>>>>>> > It looks like our previous patch messed up some logging.
>>>>>>>>>>>>> Please use this one
>>>>>>>>>>>>> > instead.
>>>>>>>>>>>>> >
>>>>>>>>>>>>> > Thanks,
>>>>>>>>>>>>> > Joao & Sarah
>>>>>>>>>>>>> >
>>>>>>>>>>>>> >
>>>>>>>>>>>>> >
>>>>>>>>>>>>> > On Wed, Mar 15, 2017 at 2:46 PM, Sarah McAlear <
>>>>>>>>>>>>> smcal...@pivotal.io> wrote:
>>>>>>>>>>>>> >>
>>>>>>>>>>>>> >> Hi Hackers!
>>>>>>>>>>>>> >>
>>>>>>>>>>>>> >> Here's a patch to move to current db migration system to
>>>>>>>>>>>>> use Alembic.
>>>>>>>>>>>>> >> Instructions to create new migrations are in the README.
>>>>>>>>>>>>> >>
>>>>>>>>>>>>> >> Thanks!
>>>>>>>>>>>>> >> Joao & Sarah
>>>>>>>>>>>>> >>
>>>>>>>>>>>>> >
>>>>>>>>>>>>> >
>>>>>>>>>>>>> >
>>>>>>>>>>>>> > --
>>>>>>>>>>>>> > Sent via pgadmin-hackers mailing list (
>>>>>>>>>>>>> pgadmin-hackers@postgresql.org)
>>>>>>>>>>>>> > To make changes to your subscription:
>>>>>>>>>>>>> > http://www.postgresql.org/mailpref/pgadmin-hackers
>>>>>>>>>>>>> >
>>>>>>>>>>>>>
>>>>>>>>>>>>>
>>>>>>>>>>>>>
>>>>>>>>>>>>> --
>>>>>>>>>>>>> Dave Page
>>>>>>>>>>>>> Blog: http://pgsnake.blogspot.com
>>>>>>>>>>>>> Twitter: @pgsnake
>>>>>>>>>>>>>
>>>>>>>>>>>>> EnterpriseDB UK: http://www.enterprisedb.com
>>>>>>>>>>>>> The Enterprise PostgreSQL Company
>>>>>>>>>>>>>
>>>>>>>>>>>>
>>>>>>>>>>>>
>>>>>>>>>>>
>>>>>>>>>>
>>>>>>>>>
>>>>>>>>
>>>>>>>
>>>>>>
>>>>>
>>>>
>>>
>>
>

Re: [pgadmin-hackers] pgAdmin4: Test result enhancement patch

[Joao Pedro De Almeida Pereira]

Hello Hackers,
We just looked into this patch and have some questions about it.

- Where is the JSON file created? Do we need to call it with some special
argument in order for it to be created?
- There is a print statement in the line 275 of runtests.py is it supposed
to be there?
- The function name addSuccess does not match the styling of the code, it
should be add_success

Suggestions:
- The definition of the class_name (run_tests.py, line 229) variable looks
the same as in the if statements below and could be extracted into a
function to avoid repeating the same code.
- In the same function when we are updating error/failure/skip test results
the code looks pretty similar and can also be extracted into a function


Thanks
Joao & Sarah

On Fri, Apr 7, 2017 at 10:15 AM, Navnath Gadakh <
navnath.gad...@enterprisedb.com> wrote:

> Hi Dave,
>
>
>   Please find the revised patch for test result enhancement, which
> include code to write passed test cases into the JSON file along with
> skipped and failed.
>
> On Tue, Apr 4, 2017 at 11:30 AM, Navnath Gadakh <
> navnath.gad...@enterprisedb.com> wrote:
>
>> Hi Dave,
>>
>>  Please find the revised patch for test result enhancement.
>>
>> What's in the patch:
>> 1. The test result summary will store in JSON file.
>> 2. Removed some redundant code from regression/test_utils.py
>> 3. Added the scenario names for feature tests.
>> 4. To print test scenario names in failed and skipped test cases, I
>> override apply_scenario() function in regression/test_utils.py
>>
>> On Mon, Apr 3, 2017 at 12:32 PM, Navnath Gadakh <
>> navnath.gad...@enterprisedb.com> wrote:
>>
>>> Hi Akshay,
>>>
>>>  Please find the revised patch for test result enhancement.
>>>
>>> *What's in the patch:*
>>> 1. The test result summary will store in JSON file.
>>> 2. Removed some redundant code from *regression/test_utils.py*
>>> *3. A*dded the scenario names for feature tests.
>>> 4. To print test scenario names in failed and skipped test cases, I o
>>> verride *apply_scenario()* function in *regression/test_utils.py*
>>>
>>> On Fri, Mar 31, 2017 at 6:16 PM, Akshay Joshi <
>>> akshay.jo...@enterprisedb.com> wrote:
>>>
 Hi Navnath

 I have run the updated patch. It is working fine with Python 2.7 but I
 am facing following error with Python 3.5, can you please look into it:

 ==

 ERROR: runTest (pgadmin.feature_tests.connect_to_server_feature_test.
 ConnectsToServerFeatureTest)

 Test database connection which can be created from the UI

 --

 Traceback (most recent call last):

   File "/Users/akshay/Development/pgadmin4/web/regression/feature_u
 tils/base_feature_test.py", line 33, in setUp

 self.page.reset_layout()

   File "/Users/akshay/Development/pgadmin4/web/regression/feature_u
 tils/pgadmin_page.py", line 33, in reset_layout

 self.click_modal_ok()

   File "/Users/akshay/Development/pgadmin4/web/regression/feature_u
 tils/pgadmin_page.py", line 38, in click_modal_ok

 self.click_element(self.find_by_xpath("//button[contains(.,'
 OK')]"))

   File "/Users/akshay/Development/pgadmin4/web/regression/feature_u
 tils/pgadmin_page.py", line 71, in find_by_xpath

 return self.wait_for_element(lambda driver: driver.
 find_element_by_xpath(xpath))

   File "/Users/akshay/Development/pgadmin4/web/regression/feature_u
 tils/pgadmin_page.py", line 128, in wait_for_element

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

   File "/Users/akshay/Development/pgadmin4/web/regression/feature_u
 tils/pgadmin_page.py", line 162, in _wait_for

 "Timed out waiting for " + waiting_for_message)

   File "/Users/akshay/Development/Workspace/lib/python3.5/site-pack
 ages/selenium/webdriver/support/wait.py", line 80, in until

 raise TimeoutException(message, screen, stacktrace)

 selenium.common.exceptions.TimeoutException: Message: Timed out
 waiting for element to exist



 ==

 ERROR: runTest (pgadmin.feature_tests.table_ddl_feature_test.
 TableDdlFeatureTest)

 Test scenarios for acceptance tests

 --

 Traceback (most recent call last):

   File "/Users/akshay/Development/pgadmin4/web/regression/feature_u
 tils/base_feature_test.py", line 33, in setUp

 self.page.reset_layout()

   File "/Users/akshay/Development/pgadmin4/web/regression/feature_u
 tils/pgadmin_page.py", line 31, in reset_layout

 self.click_element(self.find_by_partial_link_text("File

[pgadmin-hackers] Change color of selected rows on SQLEditor

[Joao Pedro De Almeida Pereira]

Hi Hackers,

>From  a visual standpoint, grey highlight makes selection look deselected
rather than selected. A brighter color would quickly provide visual
feedback on an action.

With this patch the color of selected rows look more highlighted.

Thanks
Joao


0001-Change-the-color-of-the-selected-rows-in-SQLEditor.patch
Description: Binary data

-- 
Sent via pgadmin-hackers mailing list (pgadmin-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgadmin-hackers

Re: [pgadmin-hackers][patch] Move to Alembic migration system

[Joao Pedro De Almeida Pereira]

Hello Dave and Ashesh,

Do you still need us to provide more information about this patch or is it
ready to be merged?

Thanks
Joao

On Thu, Mar 23, 2017 at 12:00 PM, Joao Pedro De Almeida Pereira <
jdealmeidapere...@pivotal.io> wrote:

> Hello Hackers,
>
> We found out a issue using Python 3 related to importing modules that we
> corrected in the patch that is now attached.
>
> Also we would like to know the status of this.
>
> Thanks
> Joao & Sarah
>
> On Fri, Mar 17, 2017 at 10:32 AM, Sarah McAlear 
> wrote:
>
>> Hi!
>>
>> We realized that this change was causing the tests to fail because the
>> folder for the sqlite databases was not being created. We also updated the
>> files to contain the missing headers.
>>
>> Thanks!
>> Joao & Sarah
>>
>>
>>
>> On Thu, Mar 16, 2017 at 9:31 AM, Dave Page  wrote:
>>
>>> Ashesh, can you review/commit this please? One thing I notice on a
>>> quick look through is that the file headers are missing everywhere.
>>> They should be present in all source files, except where they would
>>> bloat the data transfer from client to server.
>>>
>>> On Wed, Mar 15, 2017 at 8:09 PM, Sarah McAlear 
>>> wrote:
>>> > Hi Hackers!
>>> >
>>> > It looks like our previous patch messed up some logging. Please use
>>> this one
>>> > instead.
>>> >
>>> > Thanks,
>>> > Joao & Sarah
>>> >
>>> >
>>> >
>>> > On Wed, Mar 15, 2017 at 2:46 PM, Sarah McAlear 
>>> wrote:
>>> >>
>>> >> Hi Hackers!
>>> >>
>>> >> Here's a patch to move to current db migration system to use Alembic.
>>> >> Instructions to create new migrations are in the README.
>>> >>
>>> >> Thanks!
>>> >> Joao & Sarah
>>> >>
>>> >
>>> >
>>> >
>>> > --
>>> > Sent via pgadmin-hackers mailing list (pgadmin-hackers@postgresql.org)
>>> > To make changes to your subscription:
>>> > http://www.postgresql.org/mailpref/pgadmin-hackers
>>> >
>>>
>>>
>>>
>>> --
>>> Dave Page
>>> Blog: http://pgsnake.blogspot.com
>>> Twitter: @pgsnake
>>>
>>> EnterpriseDB UK: http://www.enterprisedb.com
>>> The Enterprise PostgreSQL Company
>>>
>>
>>
>

Re: [pgadmin-hackers][patch] Move to Alembic migration system

[Joao Pedro De Almeida Pereira]

Hello Hackers,

We found out a issue using Python 3 related to importing modules that we
corrected in the patch that is now attached.

Also we would like to know the status of this.

Thanks
Joao & Sarah

On Fri, Mar 17, 2017 at 10:32 AM, Sarah McAlear  wrote:

> Hi!
>
> We realized that this change was causing the tests to fail because the
> folder for the sqlite databases was not being created. We also updated the
> files to contain the missing headers.
>
> Thanks!
> Joao & Sarah
>
>
>
> On Thu, Mar 16, 2017 at 9:31 AM, Dave Page  wrote:
>
>> Ashesh, can you review/commit this please? One thing I notice on a
>> quick look through is that the file headers are missing everywhere.
>> They should be present in all source files, except where they would
>> bloat the data transfer from client to server.
>>
>> On Wed, Mar 15, 2017 at 8:09 PM, Sarah McAlear 
>> wrote:
>> > Hi Hackers!
>> >
>> > It looks like our previous patch messed up some logging. Please use
>> this one
>> > instead.
>> >
>> > Thanks,
>> > Joao & Sarah
>> >
>> >
>> >
>> > On Wed, Mar 15, 2017 at 2:46 PM, Sarah McAlear 
>> wrote:
>> >>
>> >> Hi Hackers!
>> >>
>> >> Here's a patch to move to current db migration system to use Alembic.
>> >> Instructions to create new migrations are in the README.
>> >>
>> >> Thanks!
>> >> Joao & Sarah
>> >>
>> >
>> >
>> >
>> > --
>> > Sent via pgadmin-hackers mailing list (pgadmin-hackers@postgresql.org)
>> > To make changes to your subscription:
>> > http://www.postgresql.org/mailpref/pgadmin-hackers
>> >
>>
>>
>>
>> --
>> Dave Page
>> Blog: http://pgsnake.blogspot.com
>> Twitter: @pgsnake
>>
>> EnterpriseDB UK: http://www.enterprisedb.com
>> The Enterprise PostgreSQL Company
>>
>
>


0001-Switch-to-Alembic-and-Flask-migration-db-migration-s.patch
Description: Binary data

-- 
Sent via pgadmin-hackers mailing list (pgadmin-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgadmin-hackers

[pgadmin-hackers][patch] Raise InternalServerError while retrieving table DDL

[Joao Pedro De Almeida Pereira]

Hi Hackers,

While doing the DDL patch we found out that the code was not working
properly when errors happened during SQL execution:

One example of this can be found in the file:

web/pgadmin/browser/server_groups/servers/databases/schemas/tables/__init__.py

The function '*_formatter*' that returns internal_server_error when an
error occur executing SQL
Nevertheless the '*sql*' function uses the output of '*_formatter*' during
the execution without checking it.

To solve this issue we raise an InternalServerError exception that we catch
in the '*sql*' function instead of returning an error message.


Thanks
Joao & Sarah


0001-Raise-InternalServerError-instead-of-returning-inter.patch
Description: Binary data

-- 
Sent via pgadmin-hackers mailing list (pgadmin-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgadmin-hackers

Re: [pgadmin-hackers][patch] Move test_utils.py into python_test_utils/

[Joao Pedro De Almeida Pereira]

Hi Hackers,

Sorry for not being more careful about the creation of patches and the
Python 2 vs Python 3 issue.

Thanks
Joao & Sarah

On Thu, Mar 23, 2017 at 8:00 AM, Dave Page  wrote:

> Thanks, committed. Note that this patch also touched
> copy_selected_columns_feature_test.py so I dropped that hunk.
>
> On Wed, Mar 22, 2017 at 10:01 PM, Atira Odhner  wrote:
> > Hi Hackers,
> >
> > Since we have more than one type of test suite now, it makes sense to do
> a
> > little organizing in the regression directory.
> >
> > This patch moves test_utils.py into python_test_utils/
> >
> > This patch depends on the Show DDL for Greenplum Tables patches.
> >
> > Tira & Joao
> >
> >
> > --
> > Sent via pgadmin-hackers mailing list (pgadmin-hackers@postgresql.org)
> > To make changes to your subscription:
> > http://www.postgresql.org/mailpref/pgadmin-hackers
> >
>
>
>
> --
> Dave Page
> Blog: http://pgsnake.blogspot.com
> Twitter: @pgsnake
>
> EnterpriseDB UK: http://www.enterprisedb.com
> The Enterprise PostgreSQL Company
>
>
> --
> Sent via pgadmin-hackers mailing list (pgadmin-hackers@postgresql.org)
> To make changes to your subscription:
> http://www.postgresql.org/mailpref/pgadmin-hackers
>

[pgadmin-hackers][patch] Correct syntax errors on Javascript test

[Joao Pedro De Almeida Pereira]

Hello Hackers,
We noticed that some Python comments were added to the fake_translations.js
file. We flushed out this issue running the Jasmine tests by doing(do not
forget to install node dependencies instructions in web/regression/README):

cd $PGADMIN4_SRC/web/
karma start


This patch corrects this issue.

Thanks
Joao & Sarah


0001-Change-invalid-Javascript-comments.patch
Description: Binary data

-- 
Sent via pgadmin-hackers mailing list (pgadmin-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgadmin-hackers

[pgadmin-hackers][patch] Update Selenium and Chrome driver

[Joao Pedro De Almeida Pereira]

Hello Hackers,

Chrome's latest update to version 57 is not compatible with the Chrome
driver of Selenium. This patch updates Selenium and adds the latest Chrome
driver to make the tests run.

Thanks
Joao & Sarah


0001-Update-Selenium-and-ChromeDriver-to-support-chrome-v.patch
Description: Binary data

-- 
Sent via pgadmin-hackers mailing list (pgadmin-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgadmin-hackers

[pgadmin-hackers][patch] Column selection on SQLEditor

[Joao Pedro De Almeida Pereira]

Hi Hackers,

Attached you can find the patches that finish the following Redmine Issue
https://redmine.postgresql.org/issues/2216

*Note:*
These changes depend on the acceptance of the patch that we submitted on
the email thread: "Refactor: clipboard, translations, jasmine"

This makes some changes and adds testing around the way csv is copied to
the clipboard.
We had to write a new plugin to handle selecting columns since slickGrid
does not care much about columns.

We also fixed a bug with the csv selection where strings were not being
quoted. (it was introduced recently.) Now there are jasmine tests around it.

-- 
Thanks
Joao & Tira


0001-Add-column-selector-to-SQLEditor.patch
Description: Binary data


0002-Refactor-copyData-Add-feature-test.patch
Description: Binary data


0003-Add-RangeBoundryNavigator.patch
Description: Binary data


0004-Deselect-rows-when-clicking-on-column-header.patch
Description: Binary data


0005-Update-README-with-missing-package-to-enable-testing.patch
Description: Binary data

-- 
Sent via pgadmin-hackers mailing list (pgadmin-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgadmin-hackers

[pgadmin-hackers] Introduce Migration system for SQLite database

[Joao Pedro De Almeida Pereira]

Hi Hackers,
We were looking at the migration pattern created for the SQLite database
and tried to look into the possibility of using a library that could handle
them for us.

- Migrations allow us to have a single path of creating the table instead
of creating tables using SQLAlchemy or hand rolled SQL. This pattern can
cause issues because updated databases might be different than the ones
created by SQLAlchemy
- The version numbering for the migrations is tedious and error prone.


After some research we found these 2 libraries(alembic, flask-migrate) that
can run migrations for us.

Alembic is a library written by the same person that wrote SQLAlchemy and
is used to manage and run the migrations.

Flask-migrate is the glue that joins Flask and Alembic allowing us to run
the migrations directly in the code.


What are your thoughts about this change?
Can we safely assume that everyone is in version 14 of the database?

-- 
Thanks
Joao & Sarah

Re: [pgadmin-hackers][patch] Shows titles for disabled buttons on hover

[Joao Pedro De Almeida Pereira]

Hello Dave,

We were looking into this request and we found at least one location where
the Hover does not make sense when it is disabled (the stylized "i' in the
lower left of dialogs, which has the alt-text "SQL help for this object
type".)

Could this patch be pulled in, as the buttons for which this does not apply
feel out of scope?

Thanks
George & Joao

On Mon, Mar 6, 2017 at 5:42 AM, Dave Page  wrote:

> Hi
>
> On Fri, Mar 3, 2017 at 9:19 PM, Sarah McAlear  wrote:
> > Hi Hackers,
> >
> > This patch enables hovering over disabled buttons to see their
> > functionality.
>
> This seems to work for the button bar in the query tool, but not for
> other buttons such as those on dialogues. Can you update to fix the
> problem consistently please?
>
> Note that I've created an RM for this issue:
> https://redmine.postgresql.org/issues/2226
>
> Thanks!
>
> --
> Dave Page
> Blog: http://pgsnake.blogspot.com
> Twitter: @pgsnake
>
> EnterpriseDB UK: http://www.enterprisedb.com
> The Enterprise PostgreSQL Company
>
>
> --
> Sent via pgadmin-hackers mailing list (pgadmin-hackers@postgresql.org)
> To make changes to your subscription:
> http://www.postgresql.org/mailpref/pgadmin-hackers
>



-- 
Thanks
Joao

Re: [pgadmin-hackers][patch] Test dependencies and screenshots

[Joao Pedro De Almeida Pereira]

Hi,
We adjusted the patches to correct the issues that you mentioned

Improves screenshots and reduces test flakiness

- rename screenshot files to add python version
- put screenshots into separate pg version folders

Thanks
Joao & Tira & Sarah

On Thu, Mar 2, 2017 at 5:15 AM, Dave Page  wrote:

> Hi
>
> On Wed, Mar 1, 2017 at 9:34 PM, Joao Pedro De Almeida Pereira
>  wrote:
> > Hi Hackers
> >
> > We noticed that the feature test dependencies were removed in a recent
> > patch, so we added them back.
>
> They were intentionally moved - see:
>
> https://www.postgresql.org/message-id/CA%2BOCxoxz8ZPsWBvEhSnMfq%3DW3QX-
> TaeXN9O9Gwbb%2Bf58X-9GQg%40mail.gmail.com
>
> and
>
> https://git.postgresql.org/gitweb/?p=pgadmin4.git;a=commit;h=
> 1e5de7e66e7e578a7803087389d18102e0f5d945
>
> > We also now create a screenshot with a
> > timestamp when tests fail. This should help with debugging issues in CI.
>
> Nice! There are a couple of minor issues though:
>
> - The screenshots are saving with names like:
> TemplateSelectionFeatureTest-2017-03-02 10/06/24.253843.png. Can you
> please:
>
>   * Change the / to a . or similar? Having forward slashes in
> filenames is likely to be painful (the space should probably be
> changed to an _ as well).
>   * Store the screenshots in a sub-directory named for the server
> that's currently being tested? That'll make it much easier to
> associate a screenshot with the right server in multi-server runs
> (each of my runs currently tests against 10 different servers for
> example).
>
> - I modified the ConnectToServer test to induce a failure. When I ran
> the tests (on macOS Sierra, with Python 2.7) the testsuite took
> screenshots for every test, not just the ones that failed. We should
> only store them for tests that actually did fail.
>
> Thanks, Dave.
>
> --
> Dave Page
> Blog: http://pgsnake.blogspot.com
> Twitter: @pgsnake
>
> EnterpriseDB UK: http://www.enterprisedb.com
> The Enterprise PostgreSQL Company
>



-- 
Thanks
Joao


0001-Take-screenshots-all-of-the-time-in-feature-tests.patch
Description: Binary data


0002-Create-screenshot-with-timestamp-when-tests-fail.patch
Description: Binary data


0003-Improves-screenshots-and-reduces-test-flakiness.patch
Description: Binary data

-- 
Sent via pgadmin-hackers mailing list (pgadmin-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgadmin-hackers

[pgadmin-hackers][patch] Test dependencies and screenshots

[Joao Pedro De Almeida Pereira]

Hi Hackers

We noticed that the feature test dependencies were removed in a recent
patch, so we added them back. We also now create a screenshot with a
timestamp when tests fail. This should help with debugging issues in CI.

-- 
Thanks
Joao & Tira


0001-Added-feature-test-dependencies.patch
Description: Binary data


0002-Take-screenshots-all-of-the-time-in-feature-tests.patch
Description: Binary data


0003-Create-screenshot-with-timestamp-when-tests-fail.patch
Description: Binary data

-- 
Sent via pgadmin-hackers mailing list (pgadmin-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgadmin-hackers