Thanks, applied. On Wed, Jul 19, 2017 at 2:40 PM, Surinder Kumar < surinder.ku...@enterprisedb.com> wrote:
> Hi, > > PFA patch for few code review comments given by Ashesh. > > Thanks, > Surinder > > > On Wed, Jul 19, 2017 at 4:36 PM, Surinder Kumar < > surinder.ku...@enterprisedb.com> wrote: > >> On Wed, Jul 19, 2017 at 4:27 PM, Dave Page <dp...@pgadmin.org> wrote: >> >>> That was a fun one to spot I'm sure! >>> >> Indeed, i had setup pgAdmin evn on Linux(as it works on Mac) and then i >> did `ls path/to/jquery.contextmenu.js`, it was spotted :) >> >>> >>> Thanks, committed. >>> >>> On Wed, Jul 19, 2017 at 11:21 AM, Surinder Kumar < >>> surinder.ku...@enterprisedb.com> wrote: >>> >>>> Hi >>>> >>>> Fix library path reference for `jquery.contextmenu'. This issue was >>>> only reproducible on Linux machine. >>>> So, i setup pgAdmin4 on Ubuntu VM and tested the patch and it works. >>>> >>>> Please find attached patch. >>>> >>>> Thanks, >>>> Surinder >>>> >>>> On Tue, Jul 18, 2017 at 9:05 PM, Murtuza Zabuawala < >>>> murtuza.zabuaw...@enterprisedb.com> wrote: >>>> >>>>> Great job Surinder, Load time ~2 sec on browser :) >>>>> >>>>> [image: Inline image 1] >>>>> >>>>> >>>>> -- >>>>> Regards, >>>>> Murtuza Zabuawala >>>>> EnterpriseDB: http://www.enterprisedb.com >>>>> The Enterprise PostgreSQL Company >>>>> >>>>> On Tue, Jul 18, 2017 at 9:01 PM, Dave Page <dp...@pgadmin.org> wrote: >>>>> >>>>>> Thanks, applied. >>>>>> >>>>>> On Tue, Jul 18, 2017 at 4:12 PM, Surinder Kumar < >>>>>> surinder.ku...@enterprisedb.com> wrote: >>>>>> >>>>>>> Hi >>>>>>> >>>>>>> 1. As Slickgrid has dependency of `jQuery-ui`, it was missed. now >>>>>>> added. >>>>>>> 2. Column sorting for collection nodes sometimes failing when >>>>>>> clicked on different collection nodes. >>>>>>> >>>>>>> Please find attached patch. >>>>>>> >>>>>>> Thanks >>>>>>> Surinder >>>>>>> >>>>>>> On Tue, Jul 18, 2017 at 8:20 PM, Khushboo Vashi < >>>>>>> khushboo.va...@enterprisedb.com> wrote: >>>>>>> >>>>>>>> >>>>>>>> >>>>>>>> On Tue, Jul 18, 2017 at 7:46 PM, Dave Page <dp...@pgadmin.org> >>>>>>>> wrote: >>>>>>>> >>>>>>>>> Thanks - applied! >>>>>>>>> >>>>>>>>> Awesome work - on an average of 3 tests on my Mac, load time >>>>>>>>> reduced from 11.55s with v1.6 to 5.53s with GIT Head. >>>>>>>>> >>>>>>>> Thanks to all >>>>>>> >>>>>>>> >>>>>>>>> Surinder, great work... >>>>>>>> >>>>>>>> >>>>>>>>> On Mon, Jul 17, 2017 at 5:57 PM, Surinder Kumar < >>>>>>>>> surinder.ku...@enterprisedb.com> wrote: >>>>>>>>> >>>>>>>>>> Hi >>>>>>>>>> >>>>>>>>>> Now all test cases are executing. >>>>>>>>>> Please find updated patch. >>>>>>>>>> >>>>>>>>>> Thanks >>>>>>>>>> Surinder >>>>>>>>>> >>>>>>>>>> On Mon, Jul 17, 2017 at 6:57 PM, Surinder Kumar < >>>>>>>>>> surinder.ku...@enterprisedb.com> wrote: >>>>>>>>>> >>>>>>>>>>> On Mon, Jul 17, 2017 at 4:52 PM, Dave Page <dp...@pgadmin.org> >>>>>>>>>>> wrote: >>>>>>>>>>> >>>>>>>>>>>> Hi >>>>>>>>>>>> >>>>>>>>>>>> No errors now, but do you know why JS tests are being skipped? >>>>>>>>>>>> >>>>>>>>>>> No errors/warning in console even after settings `logLevel: >>>>>>>>>>> config.LOG_DEBUG`. I am still debugging. >>>>>>>>>>> >>>>>>>>>>>> >>>>>>>>>>>> PhantomJS 2.1.1 (Mac OS X 0.0.0): Executed 4 of 216 (skipped >>>>>>>>>>>> 212) SUCCESS (0.085 secs / 0.046 secs) >>>>>>>>>>>> >>>>>>>>>>>> Thanks! >>>>>>>>>>>> >>>>>>>>>>>> On Mon, Jul 17, 2017 at 12:07 PM, Surinder Kumar < >>>>>>>>>>>> surinder.ku...@enterprisedb.com> wrote: >>>>>>>>>>>> >>>>>>>>>>>>> Hi Dave, >>>>>>>>>>>>> >>>>>>>>>>>>> I didn't removed the vendor modules when i ran regression test >>>>>>>>>>>>> cases, so modules were being referenced from vendor dir and >>>>>>>>>>>>> passed for me. >>>>>>>>>>>>> Now I have fixed path references and test cases are working. >>>>>>>>>>>>> >>>>>>>>>>>>> Please find attached patch. >>>>>>>>>>>>> >>>>>>>>>>>>> Thanks >>>>>>>>>>>>> Surinder >>>>>>>>>>>>> >>>>>>>>>>>>> On Mon, Jul 17, 2017 at 3:18 PM, Surinder Kumar < >>>>>>>>>>>>> surinder.ku...@enterprisedb.com> wrote: >>>>>>>>>>>>> >>>>>>>>>>>>>> Hi >>>>>>>>>>>>>> >>>>>>>>>>>>>> I'm currently working on first TODO: "Automatically handle >>>>>>>>>>>>>> static and template JS files" >>>>>>>>>>>>>> >>>>>>>>>>>>>> As discussed with Ashesh, currently the paths to module id >>>>>>>>>>>>>> are written manually in webpack.config.js, instead the path >>>>>>>>>>>>>> defined in >>>>>>>>>>>>>> moudle's `def get_own_javascript()` should be used. >>>>>>>>>>>>>> >>>>>>>>>>>>>> So, we will be generating a paths.json file which will >>>>>>>>>>>>>> contain: >>>>>>>>>>>>>> >>>>>>>>>>>>>> 1. resolve > alias - path with reference to module id.(Static >>>>>>>>>>>>>> files) >>>>>>>>>>>>>> >>>>>>>>>>>>>> 2. externals - list of modules to be loaded dynamically on >>>>>>>>>>>>>> demand(Template files) >>>>>>>>>>>>>> >>>>>>>>>>>>>> 3. Shim module dependency >>>>>>>>>>>>>> >>>>>>>>>>>>>> 4. List of JS modules to be loaded in specified order. >>>>>>>>>>>>>> >>>>>>>>>>>>>> *Implementation:* >>>>>>>>>>>>>> >>>>>>>>>>>>>> To generate `paths.json` file, we will be using `Flask's >>>>>>>>>>>>>> test_client` to make an http request internally within the app >>>>>>>>>>>>>> context so >>>>>>>>>>>>>> we can call `current_app.javascripts` property and return the >>>>>>>>>>>>>> list of JS >>>>>>>>>>>>>> paths and write those into paths.json file and then use it in >>>>>>>>>>>>>> webpack.shim.js before the execution of `yarn run bundle` in >>>>>>>>>>>>>> `javascript_bundler.py` >>>>>>>>>>>>>> >>>>>>>>>>>>>> *For example:* >>>>>>>>>>>>>> >>>>>>>>>>>>>> @app.route('/get_script_paths') >>>>>>>>>>>>>> def get_script_paths(): >>>>>>>>>>>>>> from flask import current_app >>>>>>>>>>>>>> from pgadmin.utils.ajax import make_json_response >>>>>>>>>>>>>> >>>>>>>>>>>>>> return make_json_response(data=current_app.javascripts) >>>>>>>>>>>>>> >>>>>>>>>>>>>> if config.DEBUG: >>>>>>>>>>>>>> with app.test_client() as client: >>>>>>>>>>>>>> import simplejson as json >>>>>>>>>>>>>> list_scripts = client.get('/get_script_paths') >>>>>>>>>>>>>> scripts = json.loads(list_scripts.data) >>>>>>>>>>>>>> >>>>>>>>>>>>>> javascriptBundler = JavascriptBundler() >>>>>>>>>>>>>> javascriptBundler.bundle(scripts['data']) >>>>>>>>>>>>>> >>>>>>>>>>>>>> >>>>>>>>>>>>>> This also needs little change in module dependency we defined >>>>>>>>>>>>>> using 'When': 'node_name' in `def get_own_javascripts(...)` >>>>>>>>>>>>>> method >>>>>>>>>>>>>> the module specified(name: module_name) is loaded when module >>>>>>>>>>>>>> given in `When` is expanded in node. Since we are using Webpack >>>>>>>>>>>>>> in which >>>>>>>>>>>>>> behaviour to load module is little different. >>>>>>>>>>>>>> >>>>>>>>>>>>>> Now in webpack we are using `imports-loader` to load specific >>>>>>>>>>>>>> modules. So this is how it should work. >>>>>>>>>>>>>> >>>>>>>>>>>>>> 1. First load all modules which do not have dependency on any >>>>>>>>>>>>>> node like 'about', 'dashboard', 'server-group', 'server' etc. >>>>>>>>>>>>>> >>>>>>>>>>>>>> 2. Load module such as `Databases` node first before its >>>>>>>>>>>>>> child nodes are loaded. >>>>>>>>>>>>>> Similarly load `Schemas` node before its child nodes are >>>>>>>>>>>>>> loaded as they are dependent on parent node. >>>>>>>>>>>>>> >>>>>>>>>>>>>> Thanks, >>>>>>>>>>>>>> Surinder >>>>>>>>>>>>>> On Wed, Jul 5, 2017 at 8:22 PM, Sarah McAlear < >>>>>>>>>>>>>> smcal...@pivotal.io> wrote: >>>>>>>>>>>>>> >>>>>>>>>>>>>>> Hello, >>>>>>>>>>>>>>> >>>>>>>>>>>>>>> >>>>>>>>>>>>>>>> *Things to discuss:* >>>>>>>>>>>>>>>> >>>>>>>>>>>>>>>> How to differentiate between a static and template JS >>>>>>>>>>>>>>>> >>>>>>>>>>>>>>>> . >>>>>>>>>>>>>>>> >>>>>>>>>>>>>>> >>>>>>>>>>>>>>> What is the advantage of webpacking templated JS? It seems >>>>>>>>>>>>>>> as though this creates a system in which the bundled >>>>>>>>>>>>>>> dependencies have to >>>>>>>>>>>>>>> refer back to the backend to load the templates. >>>>>>>>>>>>>>> >>>>>>>>>>>>>> Templated JS will not be part of generated bundle JS, they >>>>>>>>>>>>>> will load externally( an extra request will be made to server >>>>>>>>>>>>>> For example: >>>>>>>>>>>>>> translations.js) >>>>>>>>>>>>>> >>>>>>>>>>>>>>> >>>>>>>>>>>>>>> If there is a performance win in packing templated JS then >>>>>>>>>>>>>>> looking at it makes sense. Otherwise it may make sense to put >>>>>>>>>>>>>>> off until it >>>>>>>>>>>>>>> is clear that the templated files should be dealt with by either >>>>>>>>>>>>>>> de-templating them or bundling them where there is a clear >>>>>>>>>>>>>>> reason. >>>>>>>>>>>>>>> >>>>>>>>>>>>>> Template JS cannot be bundled, so i extract the <Jinja> code >>>>>>>>>>>>>> from template files and put into a separate file - ABC.js (also >>>>>>>>>>>>>> moved >>>>>>>>>>>>>> template files to static directory) and then load ABC.js >>>>>>>>>>>>>> dynamically as >>>>>>>>>>>>>> dependency of other modules. >>>>>>>>>>>>>> >>>>>>>>>>>>>>> >>>>>>>>>>>>>>> However, we're wondering about possible performance >>>>>>>>>>>>>>> penalties with templating larger files (as opposed to templating >>>>>>>>>>>>>>> on-demand.) Since jinja templates can execute arbitrary python, >>>>>>>>>>>>>>> this could >>>>>>>>>>>>>>> get time expensive and further slow things like initial >>>>>>>>>>>>>>> page-load. >>>>>>>>>>>>>>> Another concern is: what happens when a template gets out of >>>>>>>>>>>>>>> date (e.g. if browser.js had previously filled in the content >>>>>>>>>>>>>>> for >>>>>>>>>>>>>>> 'panel_item.content' and had been cached, would it render a new >>>>>>>>>>>>>>> version >>>>>>>>>>>>>>> with the new values when needed? Or is it possible that we >>>>>>>>>>>>>>> would get old >>>>>>>>>>>>>>> content?) >>>>>>>>>>>>>>> >>>>>>>>>>>>>> That file will always gets new content when loaded >>>>>>>>>>>>>> dynamically, the content is not cached. >>>>>>>>>>>>>> >>>>>>>>>>>>>>> >>>>>>>>>>>>>>> >>>>>>>>>>>>>>>> *Taks remaining:* >>>>>>>>>>>>>>>> >>>>>>>>>>>>>>>> 1. >>>>>>>>>>>>>>>> Fix local variables which are declared without using var, >>>>>>>>>>>>>>>> have to check in each file >>>>>>>>>>>>>>>> by >>>>>>>>>>>>>>>> running eslint (For now, i will fix only errors which are >>>>>>>>>>>>>>>> giving error in browser). >>>>>>>>>>>>>>>> >>>>>>>>>>>>>>>> 2. >>>>>>>>>>>>>>>> Move non-template files from ’templates’ to ’static’ >>>>>>>>>>>>>>>> directory. List of >>>>>>>>>>>>>>>> pending >>>>>>>>>>>>>>>> modules is here: >>>>>>>>>>>>>>>> >>>>>>>>>>>>>>>> - Tools (mostly all modules - 9 modules) >>>>>>>>>>>>>>>> - Browser nodes - 3 modules(resource group, roles, >>>>>>>>>>>>>>>> tablespace) >>>>>>>>>>>>>>>> - About >>>>>>>>>>>>>>>> >>>>>>>>>>>>>>>> >>>>>>>>>>>>>>>> Also can we move >>>>>>>>>>>>>>>> ' >>>>>>>>>>>>>>>> dashboard, statistic >>>>>>>>>>>>>>>> s >>>>>>>>>>>>>>>> , preferences and help >>>>>>>>>>>>>>>> ' >>>>>>>>>>>>>>>> modules inside misc to preserve modularity as pgAdmin is >>>>>>>>>>>>>>>> modular >>>>>>>>>>>>>>>> ? >>>>>>>>>>>>>>>> >>>>>>>>>>>>>>> >>>>>>>>>>>>>>> Is there anything from a organization stance you discussed >>>>>>>>>>>>>>> in the previous email that needs to be done to make this usable >>>>>>>>>>>>>>> and >>>>>>>>>>>>>>> consistent? >>>>>>>>>>>>>>> >>>>>>>>>>>>>> No >>>>>>>>>>>>>> >>>>>>>>>>>>>>> >>>>>>>>>>>>>>> >>>>>>>>>>>>>>> Thanks, >>>>>>>>>>>>>>> >>>>>>>>>>>>>>> George & Sarah >>>>>>>>>>>>>>> >>>>>>>>>>>>>> >>>>>>>>>>>>>> >>>>>>>>>>>>> >>>>>>>>>>>> >>>>>>>>>>>> >>>>>>>>>>>> -- >>>>>>>>>>>> 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 >>> >> >> > -- Dave Page Blog: http://pgsnake.blogspot.com Twitter: @pgsnake EnterpriseDB UK: http://www.enterprisedb.com The Enterprise PostgreSQL Company
