[ZEPPELIN-2519] Disable watchers not in viewport by vipul1409 · Pull Request #2326 · apache/zeppelin

vipul1409 · 2017-05-09T16:18:29Z

What is this PR for?

Currently all the watchers are enabled by default. I came across this github project https://github.com/wix/angular-viewport-watch to disable watchers not in viewport. This reduces number of watchers in notebooks with large number of paragraphs.

What type of PR is it?

[Improvement]

Todos

What is the Jira issue?

https://issues.apache.org/jira/browse/ZEPPELIN-2519

How should this be tested?

Outline the steps to test the PR here.

Screenshots (if appropriate)

Questions:

Does the licenses files need update?
Not sure of this part. This is under MIT license. Project link
Is there breaking changes for older versions?
No
Does this needs documentation?
No

1ambda · 2017-05-09T20:33:11Z

Thanks for the contribution @vipul1409

It is required to update bin_license/LICENSE when you want to add a new library.

https://github.com/apache/zeppelin/blob/master/zeppelin-distribution/src/bin_license/LICENSE#L228

Could you restart the failed jobs in travis? If they are flaky tests, it will pass otherwise you need to fix them.

https://travis-ci.org/vipul1409/zeppelin/builds/230440019

Is it possible to load angular-viewport-watcher using commonjs (webpack) instead of bower?

vi · 2017-05-09T21:46:05Z

Probably meant "Thanks for the contribution @vipul1409"

1ambda · 2017-05-09T21:52:26Z

Oops. autocompletion didn't work. Sorry for confusing.

vipul1409 · 2017-05-10T07:01:43Z

I checked Jenkins Job. The sub job failing is https://travis-ci.org/vipul1409/zeppelin/jobs/230634157 It is failing because of some git access issues. Any suggestions on what could be the issue?

1ambda · 2017-05-10T11:55:33Z

    <script src="bower_components/MathJax/MathJax.js"></script>
    <script src="bower_components/clipboard/dist/clipboard.js"></script>
    <script src="bower_components/ngclipboard/dist/ngclipboard.js"></script>
+    <script src="bower_components/scrollMonitor/scrollMonitor.js"></script>


Is it possible to use scrollMonitor without installing it? it's not added to bower.js.

1ambda · 2017-05-10T11:56:27Z

    "MathJax": "2.7.0",
-    "ngclipboard": "^1.1.1"
+    "ngclipboard": "^1.1.1",
+    "angular-viewport-watch": "^0.1.35"


It would be nicer to install angular-viewport-watch using npm instead of bower. Since we are trying to move npm from bower.

Here is one example.

[ZEPPELIN-2520] DON'T set bottom margin for markdown table #2327

vipul1409 · 2017-05-11T06:52:54Z

Addressed review comments and moved the dependency to npm. One jenkins job is failing but looks unrelated to my changes. I also see build is marked as failed in master, just double checking if master build is also broken.

vipul1409 · 2017-05-11T10:14:44Z

Even latest master is breaking with exactly same error. I am not able to figure out the issue. Any help or suggestions will be great.
https://travis-ci.org/apache/zeppelin/jobs/229852218

1ambda · 2017-05-11T13:45:46Z

 * limitations under the License.
 */

+import 'angular-viewport-watch/angular-viewport-watch.js'


Thanks for update! It would be nicer to move import stmt into app.js where the library is actually used.

1ambda · 2017-05-11T16:48:17Z

Additionally, Zeppelin supports angular API both in backend, frontend.

Could you make sure that this PR doesn't break existing features?

vipul1409 · 2017-05-16T07:28:08Z

Sorry for delayed response.
Rerun of failed job passed. I have also addressed all the PR comments.
https://travis-ci.org/vipul1409/zeppelin

1ambda · 2017-05-18T16:27:10Z

You can close and reopen this PR to trigger jenkins again (not travis).

I will test and comment soon. Thanks for updating.

1ambda · 2017-05-19T14:06:15Z

Your travis job failed at

Tests run: 1, Failures: 0, Errors: 0, Skipped: 0, Time elapsed: 125.713 sec - in org.apache.zeppelin.integration.AuthenticationIT

Results :

Tests in error: 
  InterpreterIT.testShowDescriptionOnInterpreterCreate:69 Â» ElementNotVisible El...

Tests run: 23, Failures: 0, Errors: 1, Skipped: 0

https://s3.amazonaws.com/archive.travis-ci.org/jobs/233893650/log.txt?X-Amz-Expires=30&X-Amz-Date=20170519T140437Z&X-Amz-Algorithm=AWS4-HMAC-SHA256&X-Amz-Credential=AKIAJRYRXRSVGNKPKO5A/20170519/us-east-1/s3/aws4_request&X-Amz-SignedHeaders=host&X-Amz-Signature=528cbb815c291c20ab8f4a2418e762fee0d63d63af8cbf652a1262ce8dd2ef9a

vipul1409 · 2017-05-21T05:47:19Z

Fixed all the issues and the tests have also passed. Please let me know if any more details are needed.
@1ambda

1ambda · 2017-05-23T22:07:44Z

Tested and works well (both in frontend, backend angular APIs).

LGTM.

Leemoonsoo · 2017-05-23T22:11:05Z

Thanks @vipul1409 for great contribution.
Thanks @1ambda for the review.

Merge to master if no further discussions.

1ambda · 2017-06-02T04:11:13Z

Hi @vipul1409, This PR occurs an error like

TypeError: Cannot read property 'create' of undefined
    at Object.link (http://localhost:9001/app.bundle.js:9662:52)
    at http://localhost:9001/bower_components/angular/angular.js:1240:18
    at invokeLinkFn (http://localhost:9001/bower_components/angular/angular.js:9814:9)
    at nodeLinkFn (http://localhost:9001/bower_components/angular/angular.js:9215:11)
    at compositeLinkFn (http://localhost:9001/bower_components/angular/angular.js:8510:13)
    at publicLinkFn (http://localhost:9001/bower_components/angular/angular.js:8390:30)
    at lazyCompilation (http://localhost:9001/bower_components/angular/angular.js:8728:25)
    at boundTranscludeFn (http://localhost:9001/bower_components/angular/angular.js:8527:16)
    at controllersBoundTransclude (http://localhost:9001/bower_components/angular/angular.js:9265:20)
    at ngRepeatAction (http://localhost:9001/bower_components/angular/angular.js:29607:15) <div id="{{currentParagraph.id}}_paragraphColumn_main" ng-repeat="currentParagraph in note.paragraphs" ng-controller="ParagraphCtrl" ng-init="init(currentParagraph, note)" ng-class="columnWidthClass(currentParagraph.config.colWidth)" style="margin: 0; padding: 0;" viewport-watch="" class="ng-scope" data-ng-animate="1">

When I remove viewport-watch directive from notebook.html, there is no error.
Could you fix it?

vipul1409 · 2017-06-03T00:11:37Z

Hi Kun, Raised this PR for the fix. https://github.com/apache/zeppelin/pull/2392/files Regards,

…

On Thu, Jun 1, 2017 at 9:11 PM, Kun ***@***.***> wrote: Hi @vipul1409 <https://github.com/vipul1409>, This PR occurs an error like TypeError: Cannot read property 'create' of undefined at Object.link (http://localhost:9001/app.bundle.js:9662:52) at http://localhost:9001/bower_components/angular/angular.js:1240:18 at invokeLinkFn (http://localhost:9001/bower_components/angular/angular.js:9814:9) at nodeLinkFn (http://localhost:9001/bower_components/angular/angular.js:9215:11) at compositeLinkFn (http://localhost:9001/bower_components/angular/angular.js:8510:13) at publicLinkFn (http://localhost:9001/bower_components/angular/angular.js:8390:30) at lazyCompilation (http://localhost:9001/bower_components/angular/angular.js:8728:25) at boundTranscludeFn (http://localhost:9001/bower_components/angular/angular.js:8527:16) at controllersBoundTransclude (http://localhost:9001/bower_components/angular/angular.js:9265:20) at ngRepeatAction (http://localhost:9001/bower_components/angular/angular.js:29607:15) <div id="{{currentParagraph.id}}_paragraphColumn_main" ng-repeat="currentParagraph in note.paragraphs" ng-controller="ParagraphCtrl" ng-init="init(currentParagraph, note)" ng-class="columnWidthClass(currentParagraph.config.colWidth)" style="margin: 0; padding: 0;" viewport-watch="" class="ng-scope" data-ng-animate="1"> When I remove viewport-watch directive from notebook.html, there is no error. Could you fix it? [image: image] <https://cloud.githubusercontent.com/assets/4968473/26710441/d7940e48-4794-11e7-9b1f-d16462acc49e.png> — You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub <#2326 (comment)>, or mute the thread <https://github.com/notifications/unsubscribe-auth/ABpFHmRfDYpbbu_O4gDWD4w6K9FJCgaYks5r_4tugaJpZM4NVjZF> .

-- Vipul Modi Software Development Engineer 2 Flipkart Internet Pvt. Ltd.

1ambda · 2017-07-27T05:01:05Z

Hi @vipul1409 , Could you check #2505?

vipulm added 2 commits May 9, 2017 20:59

Disable watchers not in viewport

329e33a

Trigger notification

7142578

Add license file

d698d06

1ambda reviewed May 10, 2017

View reviewed changes

vipul1409 mentioned this pull request May 10, 2017

Publish this package to npm wix-incubator/angular-viewport-watch#19

Open

Move pacakg to npm

99b19a5

1ambda reviewed May 11, 2017

View reviewed changes

Merge master

175dd17

Trigger build

b982db4

vipul1409 closed this May 19, 2017

vipul1409 reopened this May 19, 2017

vipulm added 2 commits May 21, 2017 09:10

Merging master and resolving conflicts

21c03d6

Fixing build issues

497413b

asfgit closed this in 21e702c May 25, 2017

Conversation

vipul1409 commented May 9, 2017

What is this PR for?

What type of PR is it?

Todos

What is the Jira issue?

How should this be tested?

Screenshots (if appropriate)

Questions:

Uh oh!

1ambda commented May 9, 2017 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

vi commented May 9, 2017

Uh oh!

1ambda commented May 9, 2017

Uh oh!

vipul1409 commented May 10, 2017

Uh oh!

1ambda May 10, 2017

Choose a reason for hiding this comment

Uh oh!

1ambda May 10, 2017

Choose a reason for hiding this comment

Uh oh!

vipul1409 commented May 11, 2017

Uh oh!

vipul1409 commented May 11, 2017 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

1ambda May 11, 2017 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

1ambda commented May 11, 2017 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

vipul1409 commented May 16, 2017 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

1ambda commented May 18, 2017 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

1ambda commented May 19, 2017

Uh oh!

vipul1409 commented May 21, 2017 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

1ambda commented May 23, 2017

Uh oh!

Leemoonsoo commented May 23, 2017

Uh oh!

1ambda commented Jun 2, 2017

Uh oh!

vipul1409 commented Jun 3, 2017 via email

Uh oh!

1ambda commented Jul 27, 2017

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

1ambda commented May 9, 2017 •

edited

Loading

vipul1409 commented May 11, 2017 •

edited

Loading

1ambda May 11, 2017 •

edited

Loading

1ambda commented May 11, 2017 •

edited

Loading

vipul1409 commented May 16, 2017 •

edited

Loading

1ambda commented May 18, 2017 •

edited

Loading

vipul1409 commented May 21, 2017 •

edited

Loading