When requesting builds from Data API, properties can be filtered by nam4dev · Pull Request #1739 · buildbot/buildbot

nam4dev · 2015-06-23T12:47:55Z

Currently when requesting on Data API for builds (api/v2/builds),
each item (build) returned from the JSON structure,
does not contain the "properties" field.

The drawback of it, is that one's got to request for each build its properties,
which is costly in terms of performance and network usage
(in particular on big range of data).

It should be computed and served from the back-end (Data API).

With this patch, one can request builds properties like this:

api/v2/builds/1?property=* (return the whole build's properties)
api/v2/builds/1?property=slavename&property=user (return only desired properties)
api/v2/builds?property=* (return the whole build's properties)
api/v2/builds?property=slavename&property=user (return only desired properties)

Other filters can be combined (limit, offset, order, ...)

Note: By default, none properties are returned to avoid breaking API.

Unit tests updated accordingly.
A ticket is available here: #3284

bastien31 · 2015-06-24T16:16:15Z

master/buildbot/data/builds.py

Is it useful to check dbdict? At this step, dbdict must be valid (not None) else it will crash ln 59 when requesting for "id" key, isn't it? Else need to handle the case ln 59.

sa2ajj · 2015-06-26T16:09:27Z

I'd suggest to handle the case when property is not specified (my take it that property becomes a mandatory parameter that unnecessary breaks backward compatibility).

sa2ajj · 2015-06-26T16:10:52Z

And test cases need to be take care of:

[FAIL]
Traceback (most recent call last):
  File "/home/travis/virtualenv/python2.7.9/lib/python2.7/site-packages/twisted/internet/defer.py", line 1039, in _inlineCallbacks
    result = g.send(result)
  File "/home/travis/build/buildbot/buildbot/master/buildbot/test/unit/test_data_builds.py", line 176, in do_test_event
    self.master.mq.assertProductions(exp_events)
  File "/home/travis/build/buildbot/buildbot/master/buildbot/test/fake/fakemq.py", line 89, in assertProductions
    self.testcase.assertEqual(self.productions, exp)
  File "/home/travis/virtualenv/python2.7.9/lib/python2.7/site-packages/twisted/trial/unittest.py", line 270, in assertEqual
    % (msg, pformat(first), pformat(second)))
twisted.trial.unittest.FailTest: not equal:
a = [(('builders', '10', 'builds', '1', 'new'),
  {'builderid': 10,
   'buildid': 100,
   'buildrequestid': 13,
   'buildslaveid': 20,
   'complete': False,
   'complete_at': None,
   'masterid': 824,
   'number': 1,
   'results': None,
   'started_at': datetime.datetime(1970, 1, 1, 0, 0, 1, tzinfo=tzutc()),
   'state_string': u'created'}),
 (('builds', '100', 'new'),
  {'builderid': 10,
   'buildid': 100,
   'buildrequestid': 13,
   'buildslaveid': 20,
   'complete': False,
   'complete_at': None,
   'masterid': 824,
   'number': 1,
   'results': None,
   'started_at': datetime.datetime(1970, 1, 1, 0, 0, 1, tzinfo=tzutc()),
   'state_string': u'created'})]
b = [(('builders', '10', 'builds', '1', 'new'),
  {'builderid': 10,
   'buildid': 100,
   'buildrequestid': 13,
   'buildslaveid': 20,
   'complete': False,
   'complete_at': None,
   'masterid': 824,
   'number': 1,
   'properties': {},
   'results': None,
   'started_at': datetime.datetime(1970, 1, 1, 0, 0, 1, tzinfo=tzutc()),
   'state_string': u'created'}),
 (('builds', '100', 'new'),
  {'builderid': 10,
   'buildid': 100,
   'buildrequestid': 13,
   'buildslaveid': 20,
   'complete': False,
   'complete_at': None,
   'masterid': 824,
   'number': 1,
   'properties': {},
   'results': None,
   'started_at': datetime.datetime(1970, 1, 1, 0, 0, 1, tzinfo=tzutc()),
   'state_string': u'created'})]


buildbot.test.unit.test_data_builds.Build.test_newBuildEvent
===============================================================================
[FAIL]
Traceback (most recent call last):
  File "/home/travis/virtualenv/python2.7.9/lib/python2.7/site-packages/twisted/internet/defer.py", line 1039, in _inlineCallbacks
    result = g.send(result)
  File "/home/travis/build/buildbot/buildbot/master/buildbot/test/unit/test_data_builds.py", line 78, in test_get_builder_number
    self.validateData(build)
  File "/home/travis/build/buildbot/buildbot/master/buildbot/test/util/endpoint.py", line 72, in validateData
    validation.verifyData(self, self.rtype.entityType, {}, object)
  File "/home/travis/build/buildbot/buildbot/master/buildbot/test/util/validation.py", line 655, in verifyData
    _verify(testcase, entityType, entityType.name, value)
  File "/home/travis/build/buildbot/buildbot/master/buildbot/test/util/validation.py", line 634, in _verify
    testcase.fail(msg)
twisted.trial.unittest.FailTest: build is missing keys 'property', 'properties'

buildbot.test.unit.test_data_builds.BuildEndpoint.test_get_builder_number
===============================================================================
[FAIL]
Traceback (most recent call last):
  File "/home/travis/virtualenv/python2.7.9/lib/python2.7/site-packages/twisted/internet/defer.py", line 1039, in _inlineCallbacks
    result = g.send(result)
  File "/home/travis/build/buildbot/buildbot/master/buildbot/test/unit/test_data_builds.py", line 57, in test_get_existing
    self.validateData(build)
  File "/home/travis/build/buildbot/buildbot/master/buildbot/test/util/endpoint.py", line 72, in validateData
    validation.verifyData(self, self.rtype.entityType, {}, object)
  File "/home/travis/build/buildbot/buildbot/master/buildbot/test/util/validation.py", line 655, in verifyData
    _verify(testcase, entityType, entityType.name, value)
  File "/home/travis/build/buildbot/buildbot/master/buildbot/test/util/validation.py", line 634, in _verify
    testcase.fail(msg)
twisted.trial.unittest.FailTest: build is missing keys 'property', 'properties'

buildbot.test.unit.test_data_builds.BuildEndpoint.test_get_existing
===============================================================================
[FAIL]
Traceback (most recent call last):
  File "/home/travis/virtualenv/python2.7.9/lib/python2.7/site-packages/twisted/internet/defer.py", line 1039, in _inlineCallbacks
    result = g.send(result)
  File "/home/travis/build/buildbot/buildbot/master/buildbot/test/unit/test_data_builds.py", line 109, in test_get_all
    [self.validateData(build) for build in builds]
  File "/home/travis/build/buildbot/buildbot/master/buildbot/test/util/endpoint.py", line 72, in validateData
    validation.verifyData(self, self.rtype.entityType, {}, object)
  File "/home/travis/build/buildbot/buildbot/master/buildbot/test/util/validation.py", line 655, in verifyData
    _verify(testcase, entityType, entityType.name, value)
  File "/home/travis/build/buildbot/buildbot/master/buildbot/test/util/validation.py", line 634, in _verify
    testcase.fail(msg)
twisted.trial.unittest.FailTest: build is missing keys 'property', 'properties'

buildbot.test.unit.test_data_builds.BuildsEndpoint.test_get_all
===============================================================================
[FAIL]
Traceback (most recent call last):
  File "/home/travis/virtualenv/python2.7.9/lib/python2.7/site-packages/twisted/internet/defer.py", line 1039, in _inlineCallbacks
    result = g.send(result)
  File "/home/travis/build/buildbot/buildbot/master/buildbot/test/unit/test_data_builds.py", line 116, in test_get_builder
    [self.validateData(build) for build in builds]
  File "/home/travis/build/buildbot/buildbot/master/buildbot/test/util/endpoint.py", line 72, in validateData
    validation.verifyData(self, self.rtype.entityType, {}, object)
  File "/home/travis/build/buildbot/buildbot/master/buildbot/test/util/validation.py", line 655, in verifyData
    _verify(testcase, entityType, entityType.name, value)
  File "/home/travis/build/buildbot/buildbot/master/buildbot/test/util/validation.py", line 634, in _verify
    testcase.fail(msg)
twisted.trial.unittest.FailTest: build is missing keys 'property', 'properties'

buildbot.test.unit.test_data_builds.BuildsEndpoint.test_get_builder
===============================================================================
[FAIL]
Traceback (most recent call last):
  File "/home/travis/virtualenv/python2.7.9/lib/python2.7/site-packages/twisted/internet/defer.py", line 1039, in _inlineCallbacks
    result = g.send(result)
  File "/home/travis/build/buildbot/buildbot/master/buildbot/test/unit/test_data_builds.py", line 122, in test_get_buildrequest
    [self.validateData(build) for build in builds]
  File "/home/travis/build/buildbot/buildbot/master/buildbot/test/util/endpoint.py", line 72, in validateData
    validation.verifyData(self, self.rtype.entityType, {}, object)
  File "/home/travis/build/buildbot/buildbot/master/buildbot/test/util/validation.py", line 655, in verifyData
    _verify(testcase, entityType, entityType.name, value)
  File "/home/travis/build/buildbot/buildbot/master/buildbot/test/util/validation.py", line 634, in _verify
    testcase.fail(msg)
twisted.trial.unittest.FailTest: build is missing keys 'property', 'properties'

buildbot.test.unit.test_data_builds.BuildsEndpoint.test_get_buildrequest
===============================================================================
[FAIL]
Traceback (most recent call last):
  File "/home/travis/virtualenv/python2.7.9/lib/python2.7/site-packages/twisted/internet/defer.py", line 1039, in _inlineCallbacks
    result = g.send(result)
  File "/home/travis/build/buildbot/buildbot/master/buildbot/test/unit/test_data_builds.py", line 138, in test_get_complete
    [self.validateData(build) for build in builds]
  File "/home/travis/build/buildbot/buildbot/master/buildbot/test/util/endpoint.py", line 72, in validateData
    validation.verifyData(self, self.rtype.entityType, {}, object)
  File "/home/travis/build/buildbot/buildbot/master/buildbot/test/util/validation.py", line 655, in verifyData
    _verify(testcase, entityType, entityType.name, value)
  File "/home/travis/build/buildbot/buildbot/master/buildbot/test/util/validation.py", line 634, in _verify
    testcase.fail(msg)
twisted.trial.unittest.FailTest: build is missing keys 'property', 'properties'

buildbot.test.unit.test_data_builds.BuildsEndpoint.test_get_complete
-------------------------------------------------------------------------------

nam4dev · 2015-07-01T15:55:05Z

Please, do not hesitate to review this PR and give me feedback about it :)

I'd like it to be merged if there's no problems with it.

sa2ajj · 2015-07-01T17:59:17Z

master/buildbot/data/builds.py

I think this variable is not varanted here, it just confuses.

codecov-io · 2015-07-26T19:22:04Z

Current coverage is `84.09%`

Merging #1739 into master will increase coverage by +0.02% as of 4305753

@@            master   #1739   diff @@
======================================
  Files          333     333       
  Stmts        30646   30674    +28
  Branches         0       0       
  Methods          0       0       
======================================
+ Hit          25765   25793    +28
  Partial          0       0       
  Missed        4881    4881

Review entire Coverage Diff

Uncovered Suggestions

+0.11% via ...ot/status/builder.py#379...412
+0.09% via ...ot/status/builder.py#451...479
+0.09% via ...dbot/changes/mail.py#478...505
See 7 more...

Powered by Codecov

nam4dev · 2015-07-27T08:00:19Z

@tardyp @sa2ajj Could you review this when you'll get time? thx.

tardyp · 2015-07-27T09:25:12Z

master/buildbot/data/builds.py

This part should be rather documented in the data api section of the doc. (you can add a reference to it here. dont duplicate doc.)

the appropriate doc is rtype-builds.rst

I do not see the rtype-builds.rst file in the developer section.
Should I create it or do you mean rtype-build.rst?

Yes that's what I meant

Le mar. 28 juil. 2015 19:27, Namgyal Brisson notifications@github.com a
écrit :

In master/buildbot/data/builds.py
#1739 (comment):

@@ -20,6 +20,31 @@

class Db2DataMixin(object):

def _generate_filtered_properties(self, props, filters):

"""

This method returns Build's properties according to property filters.

I do not see the rtype-builds.rst file in the developer section.
Should I create it or do you mean rtype-build.rst?

—
Reply to this email directly or view it on GitHub
https://github.com/buildbot/buildbot/pull/1739/files#r35674934.

nam4dev · 2015-07-30T08:50:25Z

@tardyp Documentation (& data/builds code) updated

sa2ajj · 2015-07-30T17:47:17Z

master/buildbot/data/resultspec.py

[nit] one line would be better and wraps -> wrap

sa2ajj · 2015-07-30T17:50:50Z

LGTM otherwise

Currently when requesting on Data API for builds (api/v2/builds), each item (build) returned from the JSON structure, does not contain the "properties" field. The drawback of it, is that one's got to request for each build its properties, which is costly in terms of performance and network usage (in particular on big range of data). It should be computed and served from the back-end (Data API). With this patch, one can request builds properties like this: - api/v2/builds/1?property=* (return the whole build's properties) - api/v2/builds/1?property=slavename&property=user (return only desired properties) - api/v2/builds?property=* (return the whole build's properties) - api/v2/builds?property=slavename&property=user (return only desired properties) Fields & Filters can be combined (limit, offset, order, ...) Important: If field(s) is/are specified properties can be retrieved only if field ``properties`` is specified. - api/v2/builds?field=buildid&field=properties&property=slavename&property=user Note: By default, none properties are returned to avoid breaking API. Unit tests & documentation updated accordingly into another commits. Signed-off-by: Namgyal Brisson <namat4css@gmail.com>

…t test) Unit tests added into, * test_data_builds.py * test_data_resultspec.py * test_www_rest.py Some minor adaptations. Signed-off-by: Namgyal Brisson <namat4css@gmail.com>

Documentation updated according to changes. Signed-off-by: Namgyal Brisson <namat4css@gmail.com>

nam4dev · 2015-07-31T12:31:10Z

@sa2ajj updated according to comments :)

tardyp · 2015-07-31T13:43:47Z

👍

When requesting builds from Data API, properties can be filtered

sa2ajj · 2015-07-31T17:59:56Z

@nam4dev Thank you very much!

nam4dev force-pushed the buildbot_builds_properties branch 3 times, most recently from 842c6cf to 516b70b Compare June 23, 2015 15:00

sa2ajj added the needs more review label Jun 24, 2015

bastien31 reviewed Jun 24, 2015
View reviewed changes

tardyp added the needs work label Jun 24, 2015

nam4dev force-pushed the buildbot_builds_properties branch from 516b70b to f1a7c2b Compare June 25, 2015 19:01

nam4dev changed the title ~~When requesting builds from Data API, properties should be returned too~~ When requesting builds from Data API, properties can be filtered Jun 25, 2015

nam4dev force-pushed the buildbot_builds_properties branch from f1a7c2b to d7bec70 Compare June 26, 2015 09:28

nam4dev force-pushed the buildbot_builds_properties branch 13 times, most recently from 16b04c3 to 1a3a2d5 Compare June 27, 2015 15:06

sa2ajj reviewed Jul 1, 2015
View reviewed changes

master/buildbot/data/builds.py Outdated

Copy link
Copy Markdown

Contributor

sa2ajj Jul 1, 2015

Choose a reason for hiding this comment

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

I think this variable is not varanted here, it just confuses.

nam4dev force-pushed the buildbot_builds_properties branch 3 times, most recently from a1fae40 to 73e58bf Compare July 26, 2015 19:02

nam4dev force-pushed the buildbot_builds_properties branch from 73e58bf to 4b785c3 Compare July 26, 2015 20:03

tardyp reviewed Jul 27, 2015
View reviewed changes

nam4dev force-pushed the buildbot_builds_properties branch from 4b785c3 to b5da75c Compare July 30, 2015 07:51

sa2ajj removed the needs work label Jul 30, 2015

sa2ajj reviewed Jul 30, 2015
View reviewed changes

sa2ajj added the needs work label Jul 30, 2015

nam4dev added 2 commits July 31, 2015 13:50

When requesting builds from Data API, properties can be filtered (uni…

07dc22a

…t test) Unit tests added into, * test_data_builds.py * test_data_resultspec.py * test_www_rest.py Some minor adaptations. Signed-off-by: Namgyal Brisson <namat4css@gmail.com>

nam4dev force-pushed the buildbot_builds_properties branch from b5da75c to 6cebb9e Compare July 31, 2015 11:50

When requesting builds from Data API, properties can be filtered (doc)

534d233

Documentation updated according to changes. Signed-off-by: Namgyal Brisson <namat4css@gmail.com>

nam4dev force-pushed the buildbot_builds_properties branch from 6cebb9e to 534d233 Compare July 31, 2015 11:59

sa2ajj removed the needs work label Jul 31, 2015

sa2ajj pushed a commit that referenced this pull request Jul 31, 2015

Merge pull request #1739 from nam4dev/buildbot_builds_properties

20e0e2c

When requesting builds from Data API, properties can be filtered

sa2ajj merged commit 20e0e2c into buildbot:master Jul 31, 2015

nam4dev deleted the buildbot_builds_properties branch August 2, 2015 09:03

tardyp mentioned this pull request Mar 1, 2017

When requesting builds from Data API, build's properties should be returned for each tardyp/buildbot_issues_sandbox#372

Open

This was referenced Mar 9, 2017

When requesting builds from Data API, build's properties should be returned for each tardyp/buildbot_issues_sandbox#549

Open

When requesting builds from Data API, build's properties should be returned for each #2939

Closed

Conversation

nam4dev commented Jun 23, 2015

Uh oh!

Choose a reason for hiding this comment

Uh oh!

sa2ajj commented Jun 26, 2015

Uh oh!

sa2ajj commented Jun 26, 2015

Uh oh!

nam4dev commented Jul 1, 2015

Uh oh!

Choose a reason for hiding this comment

Uh oh!

codecov-io commented Jul 26, 2015

Current coverage is 84.09%

Uh oh!

nam4dev commented Jul 27, 2015

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

nam4dev commented Jul 30, 2015

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

sa2ajj commented Jul 30, 2015

Uh oh!

nam4dev commented Jul 31, 2015

Uh oh!

tardyp commented Jul 31, 2015

Uh oh!

sa2ajj commented Jul 31, 2015

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

5 participants

Current coverage is `84.09%`