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

Hi

On Mon, Jun 12, 2017 at 7:22 PM, Shruti B Iyer <siyer@pivotal.io> wrote:
> Hi Dave,
>
> We regenerated the patch to add new tasks to package.json to compile react
> code for development and to not minimize it. This should fix the error you
> captured in the screenshot.
>
> The new task to lint and bundle everything for development is:
> yarn run bundle:dev
>
> The new task only bundle everything is:
> yarn run webpacker:dev
>
> We also changed the task test:feature to bundle without optimization before
> we start the tests.
>
> When we ran these commands in our machine, they did not display any error.
> Is it possible that you forgot to run yarn install before running the
> webpacker task? We are asking this because the errors look like missing node
> packages.

I did run an install, yes. This patch does work though, so applied - thanks!

Could you answer my earlier question about the need for the delay in
app_starter.py please?

Thanks.

> On Mon, Jun 12, 2017 at 12:15 PM Dave Page <dpage@pgadmin.org> wrote:
>>
>> To add to that; running the JS tests gives:
>>
>> ERROR in ./regression/javascript/history/query_history_entry_spec.jsx
>> Module not found: Error: Can't resolve 'jasmine-enzyme' in
>> '/Users/dpage/git/pgadmin4/web/regression/javascript/history'
>>  @ ./regression/javascript/history/query_history_entry_spec.jsx 13:21-46
>>
>> ERROR in ./pgadmin/static/jsx/history/query_history_entry.jsx
>> Module not found: Error: Can't resolve 'immutability-helper' in
>> '/Users/dpage/git/pgadmin4/web/pgadmin/static/jsx/history'
>>  @ ./pgadmin/static/jsx/history/query_history_entry.jsx 13:26-56
>>  @ ./regression/javascript/history/query_history_entry_spec.jsx
>>
>> ERROR in ./pgadmin/static/jsx/history/query_history_entry.jsx
>> Module not found: Error: Can't resolve 'moment' in
>> '/Users/dpage/git/pgadmin4/web/pgadmin/static/jsx/history'
>>  @ ./pgadmin/static/jsx/history/query_history_entry.jsx 17:14-31
>>  @ ./regression/javascript/history/query_history_entry_spec.jsx
>>
>> ERROR in ./regression/javascript/history/query_history_spec.jsx
>> Module not found: Error: Can't resolve 'jasmine-enzyme' in
>> '/Users/dpage/git/pgadmin4/web/regression/javascript/history'
>>  @ ./regression/javascript/history/query_history_spec.jsx 19:21-46
>> webpack: Failed to compile.
>> PhantomJS 2.1.1 (Mac OS X 0.0.0) ERROR
>>   Error: Cannot find module "immutability-helper"
>>   at regression/javascript/history/query_history_entry_spec.jsx:30705
>>
>>
>> PhantomJS 2.1.1 (Mac OS X 0.0.0) ERROR
>>   Error: Cannot find module "immutability-helper"
>>   at regression/javascript/history/query_history_spec.jsx:30705
>>
>>
>> error Command failed with exit code 1.
>> info Visit https://yarnpkg.com/en/docs/cli/run for documentation about
>> this command.
>> error Command failed with exit code 1.
>> info Visit https://yarnpkg.com/en/docs/cli/run for documentation about
>> this command.
>>
>>
>> Also, while I think of it, why the addition of the delay to
>> app_starter.py?
>>
>>
>> On Mon, Jun 12, 2017 at 5:12 PM, Dave Page <dpage@pgadmin.org> wrote:
>> > Hi,
>> >
>> > So 01 and 02 are now committed :-).
>> >
>> > 03 has a couple of problems though (likely the same):
>> >
>> > - Running the webpacker results in:
>> >
>> > (pgadmin4)piranha:web dpage$ yarn run webpacker
>> > yarn run v0.24.4
>> > $ yarn run webpack -- --optimize-minimize --config webpack.config.js
>> > yarn run v0.24.4
>> > $ "/Users/dpage/git/pgadmin4/web/node_modules/.bin/webpack"
>> > --optimize-minimize --config webpack.config.js
>> > (node:19446) 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.
>> > Hash: a5e75aa70eb6b09bdb78
>> > Version: webpack 2.3.3
>> > Time: 3983ms
>> >              Asset     Size  Chunks             Chunk Names
>> > reactComponents.js   222 kB       0  [emitted]  reactComponents
>> >         history.js  1.58 kB       1  [emitted]  history
>> >    [0] /Users/dpage/git/pgadmin4/web/~/process/browser.js 5.42 kB {0}
>> > [built]
>> >   [18] /Users/dpage/git/pgadmin4/web/~/react-dom/lib/ReactReconciler.js
>> > 6.21 kB {0} [built]
>> >   [19] /Users/dpage/git/pgadmin4/web/~/react/lib/React.js 2.69 kB {0}
>> > [built]
>> >   [31] /Users/dpage/git/pgadmin4/web/~/react/react.js 56 bytes {0}
>> > [built]
>> >   [80] ./js/history/history_collection.js 1.91 kB {1} [built]
>> >   [81] ./jsx/history/query_history.jsx 3.65 kB {0} [built]
>> >   [82] /Users/dpage/git/pgadmin4/web/~/react-dom/index.js 59 bytes {0}
>> > [built]
>> >   [83] ./js/history/index.js 690 bytes {1} [built]
>> >   [84] ./jsx/components.jsx 599 bytes {0} [built]
>> >   [85] ./jsx/history/query_history_entry.jsx 5.21 kB {0} [built]
>> >  [113] /Users/dpage/git/pgadmin4/web/~/react-dom/lib/ReactDOM.js 5.14
>> > kB {0} [built]
>> >  [175] /Users/dpage/git/pgadmin4/web/~/react/lib/ReactDOMFactories.js
>> > 5.53 kB {0} [built]
>> >  [176] /Users/dpage/git/pgadmin4/web/~/react/lib/ReactPropTypes.js
>> > 15.8 kB {0} [built]
>> >  [177] /Users/dpage/git/pgadmin4/web/~/react/lib/ReactPureComponent.js
>> > 1.32 kB {0} [built]
>> >  [178] /Users/dpage/git/pgadmin4/web/~/react/lib/ReactVersion.js 350
>> > bytes {0} [built]
>> >     + 167 hidden modules
>> >
>> > ERROR in ./jsx/history/query_history_entry.jsx
>> > Module not found: Error: Can't resolve 'immutability-helper' in
>> > '/Users/dpage/git/pgadmin4/web/pgadmin/static/jsx/history'
>> >  @ ./jsx/history/query_history_entry.jsx 13:26-56
>> >  @ ./jsx/history/query_history.jsx
>> >  @ ./jsx/components.jsx
>> >
>> > ERROR in ./jsx/history/query_history_entry.jsx
>> > Module not found: Error: Can't resolve 'moment' in
>> > '/Users/dpage/git/pgadmin4/web/pgadmin/static/jsx/history'
>> >  @ ./jsx/history/query_history_entry.jsx 17:14-31
>> >  @ ./jsx/history/query_history.jsx
>> >  @ ./jsx/components.jsx
>> > error Command failed with exit code 2.
>> > info Visit https://yarnpkg.com/en/docs/cli/run for documentation about
>> > this command.
>> > error Command failed with exit code 1.
>> > info Visit https://yarnpkg.com/en/docs/cli/run for documentation about
>> > this command
>> >
>> >
>> > - If I try to run pgAdmin, I get a script error in the UI, and console
>> > output as attached (sorry for the screenshot, I've yet to find a good
>> > way to copy/paste that info without losing the formatting).
>> >
>> > Thanks.
>> >
>> >
>> > On Mon, Jun 12, 2017 at 3:53 PM, Shruti B Iyer <siyer@pivotal.io> wrote:
>> >> Hi Hackers,
>> >>
>> >> Attached are the updated patches that apply on top of master.
>> >>
>> >> Thanks,
>> >> Shruti & Joao
>> >>
>> >>
>> >> On Mon, Jun 12, 2017 at 10:44 AM Dave Page <dpage@pgadmin.org> wrote:
>> >>>
>> >>> Hi Shruti
>> >>>
>> >>> On Mon, Jun 12, 2017 at 3:24 PM, Shruti B Iyer <siyer@pivotal.io>
>> >>> wrote:
>> >>> >
>> >>> > Hello Dave,
>> >>> >
>> >>> > Thanks for making those fixes and sharing them with us. We tried
>> >>> > applying
>> >>> > the patch and it looks like there are some missing file changes from
>> >>> > your
>> >>> > patch that were present in ours, like the Make.bat file changes. But
>> >>> > we
>> >>> > will
>> >>> > add them when we send you the new patches.
>> >>>
>> >>> Hmm, I wonder if I missed them because I applied the patch in a sub
>> >>> directory.
>> >>>
>> >>> > While trying to generate the new patches we realized some tests are
>> >>> > failing
>> >>> > in master branch due to an internal server error:
>> >>> >
>> >>> > 2017-06-12 10:04:11,938: INFO werkzeug: 127.0.0.1 - - [12/Jun/2017
>> >>> > 10:04:11]
>> >>> > "GET /browser/table/sql/1/1/12669/2200/81920 HTTP/1.1" 500 -
>> >>> > Traceback (most recent call last):
>> >>> >   File
>> >>> >
>> >>> >
>> >>> > "/Users/pivotal/.pyenv/versions/2.7.10/envs/pgadmin/lib/python2.7/site-packages/flask/app.py",
>> >>> > line 2000, in __call__
>> >>> >     return self.wsgi_app(environ, start_response)
>> >>> >   File
>> >>> >
>> >>> >
>> >>> > "/Users/pivotal/.pyenv/versions/2.7.10/envs/pgadmin/lib/python2.7/site-packages/flask/app.py",
>> >>> > line 1991, in wsgi_app
>> >>> >     response = self.make_response(self.handle_exception(e))
>> >>> >   File
>> >>> >
>> >>> >
>> >>> > "/Users/pivotal/.pyenv/versions/2.7.10/envs/pgadmin/lib/python2.7/site-packages/flask/app.py",
>> >>> > line 1567, in handle_exception
>> >>> >     reraise(exc_type, exc_value, tb)
>> >>> >   File
>> >>> >
>> >>> >
>> >>> > "/Users/pivotal/.pyenv/versions/2.7.10/envs/pgadmin/lib/python2.7/site-packages/flask/app.py",
>> >>> > line 1988, in wsgi_app
>> >>> >     response = self.full_dispatch_request()
>> >>> >   File
>> >>> >
>> >>> >
>> >>> > "/Users/pivotal/.pyenv/versions/2.7.10/envs/pgadmin/lib/python2.7/site-packages/flask/app.py",
>> >>> > line 1641, in full_dispatch_request
>> >>> >     rv = self.handle_user_exception(e)
>> >>> >   File
>> >>> >
>> >>> >
>> >>> > "/Users/pivotal/.pyenv/versions/2.7.10/envs/pgadmin/lib/python2.7/site-packages/flask/app.py",
>> >>> > line 1544, in handle_user_exception
>> >>> >     reraise(exc_type, exc_value, tb)
>> >>> >   File
>> >>> >
>> >>> >
>> >>> > "/Users/pivotal/.pyenv/versions/2.7.10/envs/pgadmin/lib/python2.7/site-packages/flask/app.py",
>> >>> > line 1639, in full_dispatch_request
>> >>> >     rv = self.dispatch_request()
>> >>> >   File
>> >>> >
>> >>> >
>> >>> > "/Users/pivotal/.pyenv/versions/2.7.10/envs/pgadmin/lib/python2.7/site-packages/flask/app.py",
>> >>> > line 1625, in dispatch_request
>> >>> >     return self.view_functions[rule.endpoint](**req.view_args)
>> >>> >   File
>> >>> >
>> >>> >
>> >>> > "/Users/pivotal/.pyenv/versions/2.7.10/envs/pgadmin/lib/python2.7/site-packages/flask/views.py",
>> >>> > line 84, in view
>> >>> >     return self.dispatch_request(*args, **kwargs)
>> >>> >   File
>> >>> > "/Users/pivotal/workspace/pgadmin4/web/pgadmin/browser/utils.py",
>> >>> > line 235, in dispatch_request
>> >>> >     return method(*args, **kwargs)
>> >>> >   File
>> >>> >
>> >>> >
>> >>> >
"/Users/pivotal/workspace/pgadmin4/web/pgadmin/browser/server_groups/servers/databases/schemas/tables/__init__.py",
>> >>> > line 315, in wrap
>> >>> >     return f(*args, **kwargs)
>> >>> >   File
>> >>> >
>> >>> >
>> >>> >
"/Users/pivotal/workspace/pgadmin4/web/pgadmin/browser/server_groups/servers/databases/schemas/tables/__init__.py",
>> >>> > line 2555, in sql
>> >>> >     data = self._formatter(did, scid, tid, data)
>> >>> >   File
>> >>> >
>> >>> >
>> >>> >
"/Users/pivotal/workspace/pgadmin4/web/pgadmin/browser/server_groups/servers/databases/schemas/tables/__init__.py",
>> >>> > line 1081, in _formatter
>> >>> >     data = self._columns_formatter(tid, data)
>> >>> >   File
>> >>> >
>> >>> >
>> >>> >
"/Users/pivotal/workspace/pgadmin4/web/pgadmin/browser/server_groups/servers/databases/schemas/tables/__init__.py",
>> >>> > line 663, in _columns_formatter
>> >>> >     column['attlen'] = matchObj.group(1)
>> >>> > AttributeError: 'NoneType' object has no attribute 'group'
>> >>> >
>> >>> > Was this introduced in a previous patch?
>> >>>
>> >>> Yes, it looks like it. For some reason it's not failing on the Jenkins
>> >>> server though. I'll ask Khushboo to fix it.
>> >>>
>> >>> > We will recreate the patches and send them ASAP.
>> >>>
>> >>> Thanks!
>> >>>
>> >>> > Thanks
>> >>> > Shruti & Joao
>> >>> >
>> >>> > On Mon, Jun 12, 2017 at 6:59 AM Dave Page <dpage@pgadmin.org> wrote:
>> >>> >>
>> >>> >> Hi
>> >>> >>
>> >>> >> OK, so Ashesh and I spend much of the morning on this.
>> >>> >>
>> >>> >> Patch 01 - Applied.
>> >>> >> Patch 02:
>> >>> >>
>> >>> >> - karma.conf.js wouldn't patch; I've manually handled that.
>> >>> >> - test-main.js wouldn't patch. The diff looked like it was trying
>> >>> >> to
>> >>> >> empty it; I have removed it instead.
>> >>> >> - The imports in pgAdmin4.py need to be made after the app root is
>> >>> >> added to the path.
>> >>> >> - The JS bundler should be in pgadmin/utils, not pgadmin/tools
>> >>> >> (which
>> >>> >> is intended for plugin modules)
>> >>> >> - The tests were failing following some changes Ashesh pushed
>> >>> >> earlier
>> >>> >> to add a client-side url_for function.
>> >>> >> - pgAdmin4.py was attempting to run the bundler on every startup.
>> >>> >> I've
>> >>> >> wrapped those called in "if config.DEBUG:" conditionals, as
>> >>> >> typically
>> >>> >> an installation for an end-user will be in a read-only directory.
>> >>> >>
>> >>> >> We've fixed all of that in the attached patch. I'm not sure why
>> >>> >> it's
>> >>> >> so much bigger than yours.
>> >>> >>
>> >>> >> The following issues are outstanding; please take a look at them:
>> >>> >>
>> >>> >> - There is no update to the Windows installer generation code
>> >>> >> (needed
>> >>> >> in 2 places unfortunately; Make.bat and Make-MinGW.bat).
>> >>> >>
>> >>> >> - The updates to the other packages call "yarn run webpacker" which
>> >>> >> is
>> >>> >> an undefined target.
>> >>> >>
>> >>> >> I haven't looked at patch 03 yet, but Ashesh did tell me it won't
>> >>> >> apply for him. Patch 4 is also untested at this stage.
>> >>> >>
>> >>> >> If the issues above can be fixed, we can get patch 2 applied then
>> >>> >> move
>> >>> >> on from there.
>> >>> >>
>> >>> >> I'll hold off on Harshal's patch for the Query Tool's load on
>> >>> >> demand
>> >>> >> to give you a chance to get this done.
>> >>> >>
>> >>> >> Thanks.
>> >>> >>
>> >>> >> On Sat, Jun 10, 2017 at 2:52 AM, George Gelashvili
>> >>> >> <ggelashvili@pivotal.io> wrote:
>> >>> >> > Hi Dave,
>> >>> >> >
>> >>> >> > Our patch touches code also changed by the patches that were
>> >>> >> > recently
>> >>> >> > committed.
>> >>> >> > That's likely what's causing this issue. We've rebased on top of
>> >>> >> > the
>> >>> >> > new
>> >>> >> > state of master.
>> >>> >> >
>> >>> >> > We had initially kept the yarn.lock .gitignored, but ran into an
>> >>> >> > issue
>> >>> >> > rather early on where an upgraded dependency introduced a
>> >>> >> > regression.
>> >>> >> > Checking-in the yarn.lock provides the "know your dependency
>> >>> >> > version"
>> >>> >> > benefit of vendorizing code without vendorization's drawback of
>> >>> >> > having
>> >>> >> > to
>> >>> >> > manually manage your dependencies.
>> >>> >> >
>> >>> >> > It is safe to delete a yarn.lock before applying a patch, as you
>> >>> >> > are
>> >>> >> > authoring master. It provides a history of the versions of each
>> >>> >> > dependency
>> >>> >> > that were working at the point in time of the commit. By
>> >>> >> > contrast,
>> >>> >> > package.json provides approximate versions and leaves room for
>> >>> >> > fixes/improvements by the dependency authors to be pulled in as
>> >>> >> > they
>> >>> >> > become
>> >>> >> > available.
>> >>> >> >
>> >>> >> > To run linter and tests:
>> >>> >> >
>> >>> >> > The tasks that Grunt used to manage are now defined as a set of
>> >>> >> > scripts
>> >>> >> > in
>> >>> >> > the package.json
>> >>> >> > After applying the patches--which may require deleting yarn.lock
>> >>> >> > for
>> >>> >> > the
>> >>> >> > first patch--you should cd web && yarn install
>> >>> >> >
>> >>> >> > Then yarn test will run the linter, jasmine specs, and python
>> >>> >> > tests
>> >>> >> > including feature tests, in that order, exiting early if there
>> >>> >> > are
>> >>> >> > failures/errors.
>> >>> >> > At the moment, the CheckForViewData test is failing on master as
>> >>> >> > well
>> >>> >> > as
>> >>> >> > in
>> >>> >> > each of these patches; that should be resolved as RM2477.
>> >>> >> >
>> >>> >> > Thanks!
>> >>> >> > George and Matt
>> >>> >> >
>> >>> >> >
>> >>> >> > On Thu, Jun 8, 2017 at 9:15 AM, Dave Page <dpage@pgadmin.org>
>> >>> >> > wrote:
>> >>> >> >>
>> >>> >> >> Hi George
>> >>> >> >>
>> >>> >> >> On Wed, Jun 7, 2017 at 10:21 PM, George Gelashvili
>> >>> >> >> <ggelashvili@pivotal.io> wrote:
>> >>> >> >> > Hi Dave,
>> >>> >> >> >
>> >>> >> >> > I split the linting out into an intermediate commit, and
>> >>> >> >> > rebased
>> >>> >> >> > on
>> >>> >> >> > top
>> >>> >> >> > of
>> >>> >> >> > master.
>> >>> >> >>
>> >>> >> >> Unfortunately, it still doesn't apply:
>> >>> >> >>
>> >>> >> >> error: patch failed: web/regression/javascript/test-main.js:1
>> >>> >> >> error: removal patch leaves file contents
>> >>> >> >> error: web/regression/javascript/test-main.js: patch does not
>> >>> >> >> apply
>> >>> >> >> Checking patch web/regression/requirements.txt...
>> >>> >> >> Checking patch web/webpack.config.js...
>> >>> >> >> Checking patch web/webpack.test.config.js...
>> >>> >> >> Checking patch web/yarn.lock...
>> >>> >> >> error: web/yarn.lock: already exists in working directory
>> >>> >> >> Applied patch .gitignore cleanly.
>> >>> >> >> Applied patch Make.bat cleanly.
>> >>> >> >> Applied patch README cleanly.
>> >>> >> >> Applied patch pkg/mac/build.sh cleanly.
>> >>> >> >> Applied patch pkg/pip/build.sh cleanly.
>> >>> >> >> Applied patch pkg/src/build.sh cleanly.
>> >>> >> >> Applied patch web/.eslintrc.js cleanly.
>> >>> >> >> Applied patch web/karma.conf.js cleanly.
>> >>> >> >> Applied patch web/package.json cleanly.
>> >>> >> >> Applied patch web/pgAdmin4.py cleanly.
>> >>> >> >> Applied patch web/pgadmin/static/jsx/components.jsx cleanly.
>> >>> >> >> Applied patch web/pgadmin/tools/javascript/__init__.py cleanly.
>> >>> >> >> Applied patch web/pgadmin/tools/javascript/javascript_bundler.py
>> >>> >> >> cleanly.
>> >>> >> >> Applied patch web/pgadmin/tools/javascript/tests/__init__.py
>> >>> >> >> cleanly.
>> >>> >> >> Applied patch
>> >>> >> >> web/pgadmin/tools/javascript/tests/test_javascript_bundler.py
>> >>> >> >> cleanly.
>> >>> >> >> Applied patch web/regression/README cleanly.
>> >>> >> >> Applied patch
>> >>> >> >> web/regression/javascript/jasmine_capture_warnings_beforeall.js
>> >>> >> >> cleanly.
>> >>> >> >> Applied patch web/regression/requirements.txt cleanly.
>> >>> >> >> Applied patch web/webpack.config.js cleanly.
>> >>> >> >> Applied patch web/webpack.test.config.js cleanly.
>> >>> >> >>
>> >>> >> >> The second (lint update) patch is even worse, with significant
>> >>> >> >> number
>> >>> >> >> change that just don't want to apply.
>> >>> >> >>
>> >>> >> >> Clearly yarn.lock needs to be removed from there repo.
>> >>> >> >>
>> >>> >> >> Once I can apply a version of this, how should I be running the
>> >>> >> >> linter
>> >>> >> >> and the unit tests?
>> >>> >> >>
>> >>> >> >> --
>> >>> >> >> Dave Page
>> >>> >> >> Blog: http://pgsnake.blogspot.com
>> >>> >> >> Twitter: @pgsnake
>> >>> >> >>
>> >>> >> >> EnterpriseDB UK: http://www.enterprisedb.com
>> >>> >> >> The Enterprise PostgreSQL Company
>> >>> >> >
>> >>> >> >
>> >>> >>
>> >>> >>
>> >>> >>
>> >>> >> --
>> >>> >> Dave Page
>> >>> >> Blog: http://pgsnake.blogspot.com
>> >>> >> Twitter: @pgsnake
>> >>> >>
>> >>> >> EnterpriseDB UK: http://www.enterprisedb.com
>> >>> >> The Enterprise PostgreSQL Company
>> >>> >>
>> >>> >> --
>> >>> >> Sent via pgadmin-hackers mailing list
>> >>> >> (pgadmin-hackers@postgresql.org)
>> >>> >> To make changes to your subscription:
>> >>> >> http://www.postgresql.org/mailpref/pgadmin-hackers
>> >>>
>> >>>
>> >>>
>> >>> --
>> >>> Dave Page
>> >>> Blog: http://pgsnake.blogspot.com
>> >>> Twitter: @pgsnake
>> >>>
>> >>> EnterpriseDB UK: http://www.enterprisedb.com
>> >>> The Enterprise PostgreSQL Company
>> >
>> >
>> >
>> > --
>> > Dave Page
>> > Blog: http://pgsnake.blogspot.com
>> > Twitter: @pgsnake
>> >
>> > EnterpriseDB UK: http://www.enterprisedb.com
>> > The Enterprise PostgreSQL Company
>>
>>
>>
>> --
>> 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