Skip to content

revamp the data module for better api and working with auto update#1916

Merged
tardyp merged 23 commits intobuildbot:masterfrom
tardyp:update
Jan 9, 2016
Merged

revamp the data module for better api and working with auto update#1916
tardyp merged 23 commits intobuildbot:masterfrom
tardyp:update

Conversation

@tardyp
Copy link
Copy Markdown
Member

@tardyp tardyp commented Nov 9, 2015

In my tests, the midterm version does not work well with updates.

I liked the collection class from the final version, so I include it back, as well as the js query implementation.

The idea is to have the collection listening to its own events and reapply the query when modified.
some of the details are described here:
https://docs.google.com/document/d/1VRdY2lqyz0UnbD4kllkzXBDIGDDoxvVgmWjn6M3oK4A/edit?ts=563b583f#

  • make the base ui work with auto update
  • update api doc
  • make this work with plugins (console/waterfall)
    • make this work with waterfall
    • make this work with console_view
  • make this work with md_base

@codecov-io
Copy link
Copy Markdown

Current coverage is 84.58%

Merging #1916 into master will not affect coverage as of f00b9bc

@@            master   #1916   diff @@
======================================
  Files          320     320       
  Stmts        31126   31129     +3
  Branches         0       0       
  Methods          0       0       
======================================
+ Hit          26328   26331     +3
  Partial          0       0       
  Missed        4798    4798       

Review entire Coverage Diff as of f00b9bc


Uncovered Suggestions

  1. +0.11% via ...ot/status/builder.py#381...414
  2. +0.09% via ...ot/status/builder.py#453...481
  3. +0.09% via ...dbot/changes/mail.py#478...505
  4. See 7 more...

Powered by Codecov. Updated on successful CI builds.

@tardyp
Copy link
Copy Markdown
Member Author

tardyp commented Dec 23, 2015

@tothandras: intermediate working version. (auto update is not finished though)
If you have any comments on how things are evolving

@tardyp
Copy link
Copy Markdown
Member Author

tardyp commented Dec 26, 2015

@rutsky 1b9b86d version has autoupdate working pretty much reliably.
force and rebuild are working, and rebuild does the all the live update until the build finishes.

I want to continue and make a big api change (simplification) before merging it.

dataService.get will not return promise, but directly the collection, as the promise is not appropriate for a constantly updating data

@tothandras
Copy link
Copy Markdown
Contributor

@tardyp You put a lot of work into this! ;) Looks good, although I didn't have time to test it.

dataService.get will not return promise, but directly the collection, as the promise is not appropriate for a constantly updating data

What about getting related data? In a lot of cases we need to know some resolved data before making the next request.

@rutsky
Copy link
Copy Markdown
Member

rutsky commented Dec 26, 2015

@tardyp, I checked your recent changes and noticed, that test fails:

27 12 2015 00:29:14.827:DEBUG [phantomjs.launcher]: unknown wrapper for Asd

LOG: 'unknown wrapper for', 'Asd'
PhantomJS 1.9.8 (Linux 0.0.0) Data utils service socketPathRE(arg) should return the WebSocket subscribe path of the parameter path FAILED
        Expected '^asd/1/bnm/[^/]+/[^/]+$' to be '^asd\/1\/bnm\/[^/]+\/[^/]+$'.
            at /home/bob/stuff/buildbot/buildbot/www/data_module/dist/tests.js:44157 <- /source/services/dataUtils/dataUtils.service.spec.coffee:56:12
            at /home/bob/stuff/buildbot/buildbot/www/data_module/dist/tests.js:44094 <- /source/services/data/data.service.spec.coffee:154:20
            at processQueue (/home/bob/stuff/buildbot/buildbot/www/data_module/dist/tests.js:14792 <- /source/angular.js:14792:0)
            at /home/bob/stuff/buildbot/buildbot/www/data_module/dist/tests.js:14808 <- /source/angular.js:14808:0
            at /home/bob/stuff/buildbot/buildbot/www/data_module/dist/tests.js:16052 <- /source/angular.js:16052:0
            at /home/bob/stuff/buildbot/buildbot/www/data_module/dist/tests.js:15870 <- /source/angular.js:15870:0
            at /home/bob/stuff/buildbot/buildbot/www/data_module/dist/tests.js:16160 <- /source/angular.js:16160:0
            at /home/bob/stuff/buildbot/buildbot/www/data_module/dist/tests.js:44096 <- /source/services/data/data.service.spec.coffee:149:30
        Expected '^asd/1/[^/]+$' to be '^asd\/1\/[^/]+$'.
            at /home/bob/stuff/buildbot/buildbot/www/data_module/dist/tests.js:44159 <- /source/services/dataUtils/dataUtils.service.spec.coffee:59:12
            at /home/bob/stuff/buildbot/buildbot/www/data_module/dist/tests.js:44094 <- /source/services/data/data.service.spec.coffee:154:20
            at processQueue (/home/bob/stuff/buildbot/buildbot/www/data_module/dist/tests.js:14792 <- /source/angular.js:14792:0)
            at /home/bob/stuff/buildbot/buildbot/www/data_module/dist/tests.js:14808 <- /source/angular.js:14808:0
            at /home/bob/stuff/buildbot/buildbot/www/data_module/dist/tests.js:16052 <- /source/angular.js:16052:0
            at /home/bob/stuff/buildbot/buildbot/www/data_module/dist/tests.js:15870 <- /source/angular.js:15870:0
            at /home/bob/stuff/buildbot/buildbot/www/data_module/dist/tests.js:16160 <- /source/angular.js:16160:0
            at /home/bob/stuff/buildbot/buildbot/www/data_module/dist/tests.js:44096 <- /source/services/data/data.service.spec.coffee:149:30

Other than that issue with test, forcescheduler works for me.

@rutsky
Copy link
Copy Markdown
Member

rutsky commented Dec 26, 2015

@tardyp, when I open waterfall view (that should contain three completed builds) I get stack overflow error:

...
equals (vendors.js:10250)
equals (vendors.js:10250)
equals (vendors.js:10250)
equals (vendors.js:10250)
equals (vendors.js:10250)
equals (vendors.js:10250)
equals (vendors.js:10250)
equals (vendors.js:10250)
equals (vendors.js:10250)
equals (vendors.js:10250)
$get.Scope.$digest (vendors.js:24888)
$get.Scope.$apply (vendors.js:25162)
(anonymous function) (d3.service.coffee:10)
fire (vendors.js:3099)
self.fireWith (vendors.js:3211)
done (vendors.js:8264)
(anonymous function) (vendors.js:8605)

equals() just calls itself recursively.

As I see equals() compares BuildInstance with other BuildInstance (that looks same to me).
The problem is, probably, that BuildInstance contains itself in it's field:

> o1
BuildInstance {_endpoint: "builds", _accessor: undefined, builderid: 1, buildid: 1, buildrequestid: 1…}
> o1._collection
[BuildInstance, BuildInstance, BuildInstance]
> o1._collection[0] === o1
true
>

Signed-off-by: Pierre Tardy <tardyp@gmail.com>
Signed-off-by: Pierre Tardy <tardyp@gmail.com>
Signed-off-by: Pierre Tardy <tardyp@gmail.com>
Signed-off-by: Pierre Tardy <tardyp@gmail.com>
Signed-off-by: Pierre Tardy <tardyp@gmail.com>
- improve data mock to add expectations
- make accessor a private method of Base data class
- default subscribe is now true if via accessor, false if via dataapi.

Signed-off-by: Pierre Tardy <tardyp@gmail.com>
Signed-off-by: Pierre Tardy <tardyp@gmail.com>
first initial load, then the updates from REST api

Signed-off-by: Pierre Tardy <tardyp@gmail.com>
Simplify the initial data gathering
get does not return promise, but a collection
lots of fixes simplification and cleanup made possible with this new api

Signed-off-by: Pierre Tardy <tardyp@gmail.com>
Signed-off-by: Pierre Tardy <tardyp@gmail.com>
@tardyp
Copy link
Copy Markdown
Member Author

tardyp commented Dec 26, 2015

@tothandras you can see the onNew, onChange callbacks that are made for dealing with loading more data, and doing stuff on the flow.

@tardyp tardyp changed the title WIP take collection back to data module revamp the data module for better api and working with auto update Dec 26, 2015
@tothandras
Copy link
Copy Markdown
Contributor

@tardyp great! 👍

It is available from db api, and very useful for the UI
for efficient way to know builder is obsolete

Signed-off-by: Pierre Tardy <tardyp@gmail.com>
and improvements:
- minimal height for builds
- only display for builders that have master attached (e.g filter out obsolete builders)
- fix +, - key bindings

Signed-off-by: Pierre Tardy <tardyp@gmail.com>
Signed-off-by: Pierre Tardy <tardyp@gmail.com>
- Unit tests should be converted to dataService.when
- build tab does not work yet

Signed-off-by: Pierre Tardy <tardyp@gmail.com>
@tardyp
Copy link
Copy Markdown
Member Author

tardyp commented Dec 29, 2015

started the port of md_base.
Having trouble with the build page @shanzi can you help?

@shanzi
Copy link
Copy Markdown
Contributor

shanzi commented Dec 30, 2015

Of course. I will have a look at it
On Wed, Dec 30, 2015 at 6:29 AM Pierre Tardy notifications@github.com
wrote:

started the port of md_base.
Having trouble with the build page @shanzi https://github.com/shanzi
can you help?


Reply to this email directly or view it on GitHub
#1916 (comment).

@shanzi
Copy link
Copy Markdown
Contributor

shanzi commented Dec 30, 2015

@tardyp It seems that the currently buildbot/data_module (whose version is 1.0.10) is being used instead of buildbot-data-js whose version is 1.1.1, isn't it?

@tardyp
Copy link
Copy Markdown
Member Author

tardyp commented Dec 30, 2015

@shanzi, by default it checks out the last published version of data module.

You will need to use the link trick in order to develop with a change data module:
https://github.com/buildbot/buildbot/blob/master/www/data_module/README.md#how-to-test-within-buildbotwwwbase-

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@tardyp I found the problem exists here. The restService successfully fetched the builds of a builder but at here they are all be filtered out.

The problems is that, in the query filter, we have a field named builderid with its value to be "2"(string). But in the array of BuildInstance, the builderid field is 2(Integer). The coffee script translate == into ===, so "2" != 2. Thus for BuildInstance this function always returns false. This causes the problem that prevent the displaying of builds in builder page.

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I make a pull request here tardyp#7
Though the list of builds is displaying now, we still have problems displaying a single build. I will look into that later.

@shanzi
Copy link
Copy Markdown
Contributor

shanzi commented Dec 30, 2015

@tardyp I have a trouble setting up data_modules's compiling environment. Gulp says

no such file or directory, scandir '/home/chase/codes/buildbot/www/data_module/node_modules/guanlecoja/lib/types'

My node version is v4.2.4 because ~v4.4.0 not found at NVM. The NPM version is v2.14.12

@rutsky
Copy link
Copy Markdown
Member

rutsky commented Dec 30, 2015

@rutsky
Copy link
Copy Markdown
Member

rutsky commented Dec 30, 2015

Try to install older version of guanlecoja, e.g. npm install guanlecoja@0.5.0

tardyp and others added 6 commits December 30, 2015 23:24
Signed-off-by: Pierre Tardy <tardyp@gmail.com>
Signed-off-by: Pierre Tardy <tardyp@gmail.com>
Signed-off-by: Pierre Tardy <tardyp@gmail.com>
@tardyp
Copy link
Copy Markdown
Member Author

tardyp commented Jan 1, 2016

Removing do not merge, as I think it is ready for merge.

md_base is "kinda" working, and is in a state were it shows its great potential.
There is still some work to do that I cannot do in a timely manner for this PR.
http://trac.buildbot.net/ticket/3397

I've updated the documentation. A notable change is that I added a dependency on blockdiag, which is a nice sphinx extension to build decent looking diagrams using text only description (kind of like graphviz, but with much better look)

http://interactive.blockdiag.com/?compression=deflate&src=eJzjUgCCpJz85OyUzMR0hWouBTBISSxJ1EtPLXEqzcxJKVbQtVNwzs_JSU0uyczPA_EQ4kkgZcElqQVYVIGFrcFG1nIBAKbsIAc

@rutsky
Copy link
Copy Markdown
Member

rutsky commented Jan 1, 2016

@tardyp Is waterfall view and console view should work? They don't in my configuration.

Force build and rebuild works good.

@tardyp
Copy link
Copy Markdown
Member Author

tardyp commented Jan 2, 2016

Yes, it is supposed to work. what is the error?

Le ven. 1 janv. 2016 à 22:08, Vladimir Rutsky notifications@github.com a
écrit :

@tardyp https://github.com/tardyp Is waterfall view and console view
should work? They don't in my configuration.

Force build and rebuild works good.


Reply to this email directly or view it on GitHub
#1916 (comment).

@rutsky
Copy link
Copy Markdown
Member

rutsky commented Jan 2, 2016

@tardyp when I open Console view (http://localhost:8080/#/console) I see infinitely spinning loader. I don't see any errors in the Firefox console.

Waterfall view works.

During Force build (and some other operations) I see ConstraintError messages in the Firefox console. If it's size constraints error it can be explained by my environment: my laptop has resolution 1366x768 and not all Buildbot UI controls are being fit in my screen.

@tardyp
Copy link
Copy Markdown
Member Author

tardyp commented Jan 2, 2016

Console view doesn't work when there is no change source. It only display
bills that are associated to changes. We should display a message when we
detect there are no data to display.

Here, the design make it difficult to distinguish no data compared to data
not yet downloaded. There is a to-do about that

Le sam. 2 janv. 2016 12:51, Vladimir Rutsky notifications@github.com a
écrit :

@tardyp https://github.com/tardyp when I open Console view (
http://localhost:8080/#/console) I see infinitely spinning loader. I
don't see any errors in the Firefox console.

Waterfall view works.

During Force build (and some other operations) I see ConstraintError
messages in the Firefox console. If it's size constraints error it can be
explained by my environment: my laptop has resolution 1366x768 and not all
Buildbot UI controls are being fit in my screen.


Reply to this email directly or view it on GitHub
#1916 (comment).

@rutsky rutsky mentioned this pull request Jan 2, 2016
@tardyp
Copy link
Copy Markdown
Member Author

tardyp commented Jan 7, 2016

I'd like to merge this by end of day if there are no objection.

tardyp added a commit that referenced this pull request Jan 9, 2016
revamp the data module for better api and working with auto update
@tardyp tardyp merged commit 1b1a428 into buildbot:master Jan 9, 2016
@tardyp tardyp deleted the update branch April 24, 2016 13:42
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants