public inbox for [email protected]
help / color / mirror / Atom feedFrom: Dave Page <[email protected]>
To: Shruti B Iyer <[email protected]>
Cc: Surinder Kumar <[email protected]>
Cc: Joao Pedro De Almeida Pereira <[email protected]>
Cc: Murtuza Zabuawala <[email protected]>
Cc: pgadmin-hackers <[email protected]>
Subject: Re: [pgAdmin4] [PATCH] History Tab rewrite in React
Date: Mon, 12 Jun 2017 17:12:04 +0100
Message-ID: <CA+OCxozwJsocmphyejzF1iMSJR7+h5bYzwT4GFDRcf7qtFQ_2g@mail.gmail.com> (raw)
In-Reply-To: <CACrUwhJdAefbqff7VT8E6WR=+SZH-JcomH4DbVA_nwwaO1672A@mail.gmail.com>
References: <CAFS4TJYDRi6fs+wrhkj89gFe3xaFOkoApCK+AU2O4DfU2XfXog@mail.gmail.com>
<CAKKotZQSTtJVkBKMeFi9vTrPugBEbEE38dhbbn-hr+hpd0Qzbw@mail.gmail.com>
<CA+OCxowChMtZzaSwrG5Sganh4LiMQcgL7BpoqhTxZga30DqDXw@mail.gmail.com>
<CAKKotZRmvbScMdUtjMgUJdRe-EXaqDoDbYBxrMZrkbVDVR_iUg@mail.gmail.com>
<CA+OCxow3e=q9w_e=Kkw1ebZJ82EM8RyFmiTaXihdqhX6HbzZhw@mail.gmail.com>
<CAFS4TJapVubRP_BK01GwYjfN7zpaTbCMX+CROC4mV93aL08DOg@mail.gmail.com>
<CA+OCxozPDpP_5zdLjOSQhiGHgzE-QDsFv8-WyC8-XTxf5+uSJg@mail.gmail.com>
<CAE+jjakzXbzvtai0Z2q0bXfecMhDzZwtn+p8mQwV7kfRGPON_Q@mail.gmail.com>
<CA+OCxoxkM--KjFWt7FkJm+mrL5sA9jdZ59YN7=n=2ZMrkH8iSg@mail.gmail.com>
<CAFS4TJaZA0U_BtiUBdx1jT6Z5bQzMQbjhwrjimDLgfkTb=gWJg@mail.gmail.com>
<CAE+jjakrsf7f41LyCy6BWvjrsH9FyKK98koO0MgUkR0qmbk-uw@mail.gmail.com>
<CA+OCxoxH+5gAjw17rWQb-jqNBeQGgOs4Q=GZ7KEwdmGHOQrsBw@mail.gmail.com>
<CAHowoHZJtKn8F9XuiK2yFT=_OcgG7Ctx83iiFab17PwexajfDQ@mail.gmail.com>
<CA+OCxozz7pbMQi_h-USovgwA2XkVzpeAUuK-zcro4La-_KjgtA@mail.gmail.com>
<CAFS4TJYRuaRkHguU_BkMAa4vv7Xy_aa37B4M4HSbJ0h2QpmcFA@mail.gmail.com>
<CA+OCxozdD8_4uyu9f+CJTjtH0x8bcKoeNMtRDcSHXSKStAPwbQ@mail.gmail.com>
<CAE+jjakmryMDovhh+OMsw=FSTj5W62CuhbdQS4WpjcLSs9jO4g@mail.gmail.com>
<CAM5-9D-wS-9qwMwvpUCwc8s=KumY+k8pkM7tOt1eysuFvq3n-w@mail.gmail.com>
<CAFS4TJZtCDqoenHREPC7PEopwYVyPGUh0_9WtxQKNV2w_WLAjw@mail.gmail.com>
<CAM5-9D9cm88KaiNjAwyNQG7+0FNxcAEJ6xaWTS9+pJ-6uLLTWQ@mail.gmail.com>
<CAGRPzo-byoSsREPZxJ-bEbse2c2emFF5UMD=SGCJHWduk8kiCA@mail.gmail.com>
<CA+OCxox3vW4dpK5w9LuL=axXpp_xR5V14oLbGYoe4N9zSOP3dA@mail.gmail.com>
<CAHowoHa-xGTQgckQC431jusaHD2Eg3S0KCrn24yt9uiS0ow7bA@mail.gmail.com>
<CA+OCxoy3gVbGiPL3hi250Ya0qMOURYOERH5XxYrnrt2Co2v-xw@mail.gmail.com>
<CAHowoHZFkDQrMO0-e2gx3ffo3kB56Qwb3jT2S1M7Sc8Bpo0Usw@mail.gmail.com>
<CA+OCxoxtxBO-mWfUokBu1WG8VH4E6MrHj16mrKT=L0svYvKfGg@mail.gmail.com>
<CACrUwhJ_YXJuZGbBrMKrCTq5qQ3JJ8zrpTu_fauOC1KXcmADNg@mail.gmail.com>
<CA+OCxowHBoAMBE9Np--JupNneYGe__=4Zwm2JMSPGD2U6=S2Lg@mail.gmail.com>
<CACrUwhJdAefbqff7VT8E6WR=+SZH-JcomH4DbVA_nwwaO1672A@mail.gmail.com>
List-Unsubscribe: <mailto:[email protected]?body=unsub%20pgadmin-hackers>
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 <[email protected]> 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 <[email protected]> wrote:
>>
>> Hi Shruti
>>
>> On Mon, Jun 12, 2017 at 3:24 PM, Shruti B Iyer <[email protected]> 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 <[email protected]> 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
>> >> <[email protected]> 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 <[email protected]> wrote:
>> >> >>
>> >> >> Hi George
>> >> >>
>> >> >> On Wed, Jun 7, 2017 at 10:21 PM, George Gelashvili
>> >> >> <[email protected]> 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 ([email protected])
>> >> 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
--
Sent via pgadmin-hackers mailing list ([email protected])
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgadmin-hackers
Attachments:
[image/png] Screen Shot 2017-06-12 at 17.10.11.png (191.2K, 2-Screen%20Shot%202017-06-12%20at%2017.10.11.png)
download | view image
view thread (45+ messages) latest in thread
reply
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Reply to all the recipients using the --to and --cc options:
reply via email
To: [email protected]
Cc: [email protected], [email protected], [email protected], [email protected], [email protected]
Subject: Re: [pgAdmin4] [PATCH] History Tab rewrite in React
In-Reply-To: <CA+OCxozwJsocmphyejzF1iMSJR7+h5bYzwT4GFDRcf7qtFQ_2g@mail.gmail.com>
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
This inbox is served by agora; see mirroring instructions
for how to clone and mirror all data and code used for this inbox