Re: [review-request] adamian/20150603_remove_xhr_review2 - REST refactoring

[Michael Wood <michael.g.wood@intel.com>]

d3faa0ec50d84f3f7893e7c88b40993b021b4707 
<http://git.yoctoproject.org/cgit/cgit.cgi/poky-contrib/commit/?h=adamian/20150603_remove_xhr_review2&id=d3faa0ec50d84f3f7893e7c88b40993b021b4707>


diff --git a/bitbake/lib/toaster/toastergui/static/js/table.js 
b/bitbake/lib/toaster/toastergui/static/js/table.js
index 7588a4a..80e9ec2 100644
--- a/bitbake/lib/toaster/toastergui/static/js/table.js 
<http://git.yoctoproject.org/cgit/cgit.cgi/poky-contrib/tree/bitbake/lib/toaster/toastergui/static/js/table.js?h=adamian/20150603_remove_xhr_review2&id=6a9a3995803334ef6a9b9eb238855f5bdb827759>
+++ b/bitbake/lib/toaster/toastergui/static/js/table.js 
<http://git.yoctoproject.org/cgit/cgit.cgi/poky-contrib/tree/bitbake/lib/toaster/toastergui/static/js/table.js?h=adamian/20150603_remove_xhr_review2&id=d3faa0ec50d84f3f7893e7c88b40993b021b4707>
@@ -113,8 +113,20 @@ function tableInit(ctx){
     for (var i in tableData.rows){
         var row = $("<tr></tr>");
         for (var key_j in tableData.rows[i]){
+     /* if we have a static: version of a key, prefer the static: 
version for rendering */
+     var orig_key_j = key_j;


I'm not keen on having data for the table widget and data for the API 
mixed up in the same table/data response. A different definition of 
table should be used if it's for a different purpose.

diff --git a/bitbake/lib/toaster/toastergui/views.py 
b/bitbake/lib/toaster/toastergui/views.py
index 91c4fa2..280159a 100755
--- a/bitbake/lib/toaster/toastergui/views.py
+++ b/bitbake/lib/toaster/toastergui/views.py
@@ -87,6 +87,35 @@ def _project_recent_build_list(prj):
list(prj.buildrequest_set.filter(state__in=[BuildRequest.REQ_COMPLETED, 
BuildRequest.REQ_FAILED]).order_by("-pk")[:3]))


+
+def objtojson(obj):

Could you comment on this function it's not obvious why it's needed or 
what it does



732d175ded8517cd6dee02d441f318c775aaba8b 
<http://git.yoctoproject.org/cgit/cgit.cgi/poky-contrib/commit/?h=adamian/20150603_remove_xhr_review2&id=732d175ded8517cd6dee02d441f318c775aaba8b>

diff --git a/bitbake/lib/toaster/toastergui/widgets.py 
b/bitbake/lib/toaster/toastergui/widgets.py
index 82b7514..407a0fb 100644
--- a/bitbake/lib/toaster/toastergui/widgets.py 
<http://git.yoctoproject.org/cgit/cgit.cgi/poky-contrib/tree/bitbake/lib/toaster/toastergui/widgets.py?h=adamian/20150603_remove_xhr_review2&id=d3faa0ec50d84f3f7893e7c88b40993b021b4707>
+++ b/bitbake/lib/toaster/toastergui/widgets.py 
<http://git.yoctoproject.org/cgit/cgit.cgi/poky-contrib/tree/bitbake/lib/toaster/toastergui/widgets.py?h=adamian/20150603_remove_xhr_review2&id=732d175ded8517cd6dee02d441f318c775aaba8b>
...

@@ -289,8 +299,11 @@ class ToasterTable(TemplateView):
col['field_name'] = col['static_data_name']
- if True: # we add the raw model data at all times
- model_data = row
+ # compute the computation on the raw data if needed
+ model_data = row
+ if col['computation']:
     + model_data = col['computation'](row)
+ else:


I don't see why there should be a case where you're computing data on 
each row, if you're having to do this then I think the model is wrong.

couldn't this be done with a filter?

+ self.add_column(title="Project compatible Layer ID",
+ displayable = False,
+ field_name = "projectcompatible_layer",
+ computation = lambda x: 
(x.layer_version.get_equivalents_wpriority(Project.objects.get(pk=kwargs['pid']))[0]))
+

It's pretty confusing when looking at the JSON response, you get lots of 
other objects included even though you asked for recipe objects, infact 
I counted 16 different objects referenced in the JSON response for 
recipes (e.g. 
http://127.0.0.1:9999/toastergui/project/2/recipes/?format=json ) I'm 
not sure why.



bb507c35ae1af626681dcb666491f6ea97ea31f5 
<http://git.yoctoproject.org/cgit/cgit.cgi/poky-contrib/commit/?h=adamian/20150603_remove_xhr_review2&id=bb507c35ae1af626681dcb666491f6ea97ea31f5>


diff --git a/bitbake/lib/toaster/toastergui/widgets.py 
b/bitbake/lib/toaster/toastergui/widgets.py
index 8bc3d7f..f5a1b3e 100644
--- a/bitbake/lib/toaster/toastergui/widgets.py 
<http://git.yoctoproject.org/cgit/cgit.cgi/poky-contrib/tree/bitbake/lib/toaster/toastergui/widgets.py?h=adamian/20150603_remove_xhr_review2&id=73d734fd193b2fe591b9407038da5d7d6db0fabf>
+++ b/bitbake/lib/toaster/toastergui/widgets.py 
<http://git.yoctoproject.org/cgit/cgit.cgi/poky-contrib/tree/bitbake/lib/toaster/toastergui/widgets.py?h=adamian/20150603_remove_xhr_review2&id=bb507c35ae1af626681dcb666491f6ea97ea31f5>
@@ -253,8 +253,8 @@ class ToasterTable(TemplateView):
data = cache.get(cache_name)
- if data:
- return data
+ #if data:
+ # return data


?


diff --git a/bitbake/lib/toaster/toastermain/settings.py 
b/bitbake/lib/toaster/toastermain/settings.py
index 225138b..3c7cb3b 100644
--- a/bitbake/lib/toaster/toastermain/settings.py 
<http://git.yoctoproject.org/cgit/cgit.cgi/poky-contrib/tree/bitbake/lib/toaster/toastermain/settings.py?h=adamian/20150603_remove_xhr_review2&id=73d734fd193b2fe591b9407038da5d7d6db0fabf>
+++ b/bitbake/lib/toaster/toastermain/settings.py 
<http://git.yoctoproject.org/cgit/cgit.cgi/poky-contrib/tree/bitbake/lib/toaster/toastermain/settings.py?h=adamian/20150603_remove_xhr_review2&id=bb507c35ae1af626681dcb666491f6ea97ea31f5>
@@ -223,7 +223,7 @@ CACHES = {
'default': {
'BACKEND': 'django.core.cache.backends.filebased.FileBasedCache',
'LOCATION': '/tmp/django-default-cache',
- 'TIMEOUT': 5,
+ 'TIMEOUT': 1,
}
}

1 second seems a bit too short to me..


a52772e243680d46836555b3eec805b7eb1d66d4 
<http://git.yoctoproject.org/cgit/cgit.cgi/poky-contrib/commit/?h=adamian/20150603_remove_xhr_review2&id=a52772e243680d46836555b3eec805b7eb1d66d4>


diff --git a/bitbake/lib/toaster/toastergui/static/js/base.js 
b/bitbake/lib/toaster/toastergui/static/js/base.js
index 747442c..5efe458 100644
--- a/bitbake/lib/toaster/toastergui/static/js/base.js
+++ b/bitbake/lib/toaster/toastergui/static/js/base.js
@@ -21,7 +21,15 @@ function basePageInit (ctx) {

+
+          /* we can build this project; enable input fields */
+          newBuildTargetInput.prop("disabled", false);
+          newBuildTargetBuildBtn.prop("disabled", false);
+
+          libtoaster.makeTypeahead(newBuildTargetInput, 
libtoaster.ctx.projectTargetsUrl, { format: "json" }, function(item){
+            /* successfully selected a target */
+            selectedTarget = item;
+            });
+
          }
      }, null);


Minor white space issues here.

31f7cef51041d7244dbf95a5258e45cbbf851f43 
<http://git.yoctoproject.org/cgit/cgit.cgi/poky-contrib/commit/?h=adamian/20150603_remove_xhr_review2&id=31f7cef51041d7244dbf95a5258e45cbbf851f43>

diff --git a/bitbake/lib/toaster/toastergui/static/js/base.js 
b/bitbake/lib/toaster/toastergui/static/js/base.js
index ccc23e0..9424b6c 100644
--- a/bitbake/lib/toaster/toastergui/static/js/base.js 
<http://git.yoctoproject.org/cgit/cgit.cgi/poky-contrib/tree/bitbake/lib/toaster/toastergui/static/js/base.js?h=adamian/20150603_remove_xhr_review2&id=c93f33414004d6e6f2040c226ac3488d5f8c6125>
+++ b/bitbake/lib/toaster/toastergui/static/js/base.js 
<http://git.yoctoproject.org/cgit/cgit.cgi/poky-contrib/tree/bitbake/lib/toaster/toastergui/static/js/base.js?h=adamian/20150603_remove_xhr_review2&id=31f7cef51041d7244dbf95a5258e45cbbf851f43>
@@ -15,20 +15,21 @@ function basePageInit (ctx) {


+      /* we set the effective context of the page to the currently 
selected project */
+      /* TBD: do we override even if we already have a context project 
?? */
+      /* TODO: replace global library context with references to the 
"selected" project */
+      libtoaster.ctx.projectPageUrl = selectedProject.projectPageUrl;
+      libtoaster.ctx.xhrProjectEditUrl = selectedProject.xhrProjectEditUrl;
+      libtoaster.ctx.projectName = selectedProject.name;
+      libtoaster.ctx.projectId = selectedProject.id;


This creates strange behaviour if you're on a page which has actions 
such as build or add/remove layer. If you're on project 1 and open the 
New build button and select a project, suddenly all the buttons on the 
page you're on act on a different project from the one you're viewing.

Michael


On 09/06/15 11:58, Barros Pena, Belen wrote:
> Sorry: caching playing tricks on me.
>
> All is working, except
>
> * In the all compatible recipes page, when we show a commit in the
> Revision column, the full SHA is showing, instead of the button with the
> first characters of it. This is working properly in the all layers page,
> by the way.
>
>
> Which we can fix at a later stage.
>
> Thanks!
>
> Belén
>
> On 09/06/2015 11:02, "Barros Pena, Belen" <belen.barros.pena@intel.com>
> wrote:
>
>> I've changed the subject to the branch we need to review, so that we don't
>> get confused.
>>
>> On 08/06/2015 18:44, "Damian, Alexandru" <alexandru.damian@intel.com>
>> wrote:
>>
>>> Sent too soon -
>>>
>>>
>>> adamian/20150603_remove_xhr_review2
>> There are still a few things not working for me:
>>
>> * The autocomplete functionality is still not working for me in the
>> project page, but it seems to work in the 'new build' button
>>
>> * In the all compatible recipes page, when we show a commit in the
>> Revision column, the full SHA is showing, instead of the button with the
>> first characters of it. This is working properly in the all layers page,
>> by the way.
>>
>> * Builds are not starting: I've tried from several pages, but when I click
>> the build button, the build does not start.
>>
>> * I cannot change the project release: when I click the 'save' button I
>> get the following error
>>
>> Release matching query does not exist.
>> Traceback (most recent call last):
>>   File "/home/yocto/master/bitbake/lib/toaster/toastergui/views.py", line
>> 2337, in xhr_datatypeahead
>>     lv = prj.compatible_layerversions(release =
>> Release.objects.get(pk=request.GET.get('search',
>> None))).filter(layer__name = i.layercommit.layer.name)
>>   File
>> "/usr/local/lib/python2.7/dist-packages/django/db/models/manager.py", line
>> 151, in get
>>     return self.get_queryset().get(*args, **kwargs)
>>   File "/usr/local/lib/python2.7/dist-packages/django/db/models/query.py",
>> line 307, in get
>>     self.model._meta.object_name)
>> DoesNotExist: Release matching query does not exist
>>
>> Thanks!
>>
>> Belén
>>
>>
>>>
>>>
>>> Can you guys please review ?
>>>
>>>
>>> If ok, I'm going to submit.
>>>
>>>
>>> Alex
>>>
>>>
>>> On Mon, Jun 8, 2015 at 6:43 PM, Damian, Alexandru
>>> <alexandru.damian@intel.com> wrote:
>>>
>>> Hi,
>>>
>>>
>>> I think I'm done with the URL refactoring - the patchset is here
>>>
>>>
>>>
>>>
>>> On Thu, Jun 4, 2015 at 10:25 AM, Barros Pena, Belen
>>> <belen.barros.pena@intel.com> wrote:
>>>
>>> Hi Alex,
>>>
>>> I've had a quick look at this branch. Table headings are back :) but
>>> found
>>> a couple of things:
>>>
>>> * the layer dependencies check is not working. When I add a layer with
>>> dependencies, I am not prompted to add them.
>>> * the autocomplete functionality is not working. I always get the same
>>> suggestions independently of what I type
>>> * the new build button in pages belonging to a project shows the recipe
>>> text field and build buttons disabled, even though there is a project
>>> selected.
>>> * trying to change the project release throws the following error:
>>>
>>> Release matching query does not exist.
>>> Traceback (most recent call last):
>>>   File "/home/yocto/master/bitbake/lib/toaster/toastergui/views.py",
>>> line 2393, in xhr_datatypeahead
>>>     lv = prj.compatible_layerversions(release =
>>> Release.objects.get(pk=request.GET.get('search',
>>> None))).filter(layer__name =
>>> i.layercommit.layer.name <http://i.layercommit.layer.name>)
>>>   File
>>> "/usr/local/lib/python2.7/dist-packages/django/db/models/manager.py",
>>> line 151, in get
>>>     return self.get_queryset().get(*args, **kwargs)
>>>   File
>>> "/usr/local/lib/python2.7/dist-packages/django/db/models/query.py",
>>> line 307, in get
>>>     self.model._meta.object_name)
>>> DoesNotExist: Release matching query does not exist.
>>>
>>> The rest seems to be working ok, as far as I can see.
>>>
>>>
>>> Thanks!
>>>
>>> On 03/06/2015 15:35, "Damian, Alexandru" <alexandru.damian@intel.com>
>>> wrote:
>>>
>>>> Thanks -
>>>>
>>>>
>>>> I'm pushing new patches to bring more parts of the system in conformance
>>>> - just removed xhr_project(info|edit) in favor of JSON-based "project"
>>>> page.
>>>>
>>>>
>>>> I will update on this thread when I tackled the bugs highlighted on the
>>>> other thread, and when the new functionality is ready to merge from my
>>>> POV.
>>>>
>>>>
>>>> Alex
>>>>
>>>>
>>>> On Wed, Jun 3, 2015 at 2:59 PM, Michael Wood
>>>> <michael.g.wood@intel.com> wrote:
>>>>
>>>>
>>>> 8305ee985bdc87dcfb63be4e0ddafb0a9239ffd4
>>>>
>>>> Looks good to me.
>>>>
>>>> Yeah that's a good idea to make ToasterTable inherit from TemplateView
>>>> rather than have it in between.
>>>>
>>>> Thanks,
>>>>
>>>> Michael
>>>>
>>>>
>>>>
>>>> On 03/06/15 14:15, Damian, Alexandru wrote:
>>>>
>>>> Hi,
>>>>
>>>> I've progressed a bit with the REST refactoring, I've tackled the
>>>> ToasterTables. Thanks to good quality of the existing code , changing
>>>> the
>>>> layout of the URL was straightforward.
>>>>
>>>> adamian/20150515_fix_table_header
>>>>
>>>> This reduces the number of entry points for the URL, removes the ugly
>>>> hardcoding of XHR_ table urls in tables.py, and makes sure the APIs are
>>>> both human- and machine-readable.
>>>>
>>>> I'll continue with refactoring to remove the xhr_datatypeahead and
>>>> similar URL endpoints which should not have existed in the first place.
>>>>
>>>> Cheers,
>>>> Alex
>>>>
>>>> --
>>>> Alex Damian
>>>> Yocto Project
>>>> SSG / OTC
>>>>
>>>>
>>>>
>>>>
>>>>
>>>>
>>>>
>>>> ---------------------------------------------------------------------
>>>> Intel Corporation (UK) Limited
>>>> Registered No. 1134945 (England)
>>>> Registered Office: Pipers Way, Swindon SN3 1RJ
>>>> VAT No: 860 2173 47
>>>>
>>>> This e-mail and any attachments may contain confidential material for
>>>> the sole use of the intended recipient(s). Any review or distribution
>>>> by others is strictly prohibited. If you are not the intended
>>>> recipient, please contact the sender and delete all copies.
>>>>
>>>>
>>>>
>>>>
>>>>
>>>>
>>>>
>>>>
>>>> --
>>>> Alex Damian
>>>> Yocto Project
>>>>
>>>> SSG / OTC
>>>>
>>>>
>>>>
>>>
>>>
>>>
>>>
>>>
>>>
>>>
>>>
>>>
>>> -- 
>>> Alex Damian
>>> Yocto Project
>>>
>>> SSG / OTC
>>>
>>>
>>>
>>>
>>>
>>>
>>>
>>>
>>>
>>>
>>>
>>>
>>> -- 
>>> Alex Damian
>>> Yocto Project
>>>
>>> SSG / OTC
>>>
>>>
>>>