Hello Alex,
Thank you for these updates, the code looks really solid !
Here are a couple of suggestions I have for these change-sets:
For the commit: *bitbake: runqueue, build, dsi: event data change**
***
+ task.save()
+
+ if isinstance(event, bb.build.TaskBase):
+ task.recipe.name = event._package
+ task.recipe.save()
The first save() can go away, as there is another task.save() call a few
lines down.
-----
Would it be better to change this:
self._message = "recipe %s: task %s: %s" % (d.getVar("PF", True),
t, self.getDisplayName())
into this:
self._message = "recipe %s: task %s: %s" % (self._package, t,
self.getDisplayName()) ?
------
identifier = task_information['recipe'].file_path +
task_information['task_name']
This may be a very long key. Maybe we can do something like this:
identifier = task_information['recipe'].file_path.split('/')[-1] +
task_information['task_name']
This will store only the recipe name+version together with the task_name
If this point is taken, this line:
identifier = event.taskfile + event.taskname
Should become:
identifier = event.taskfile.split('/')[-1] + event.taskname
! Beware of other lines that may need to change together with this.
=====================================================
For commit:***bitbake: dsi: refactor the BuildInfoHelper code*
- return machine_info
+ pass
def create_machine_object(self, machine_information):
Do we need this 'pass' here ?
---
+ task_object.outcome=task_information['outcome']
+
+ task_object.task_executed=task_information['task_executed']
Can we delete the space between these lines ? It makes the first line
stand out from the rest and there's no reason for that.
---
+ import traceback
It's best to have all imports at the beginning of the file.
-----
Get task object and get recipe object are not consistent with eachother.
get_recipe should consider the created attribute before trying to store
information:
+ try:
+ recipe_object.name=recipe_information['name']
+ recipe_object.version=recipe_information['version']
+ recipe_object.summary=recipe_information['summary']
+ recipe_object.description=recipe_information['description']
+ recipe_object.section=recipe_information['section']
+ recipe_object.license=recipe_information['license']
+ recipe_object.licensing_info=recipe_information['licensing_info']
+ recipe_object.homepage=recipe_information['homepage']
+ recipe_object.bugtracker=recipe_information['bugtracker']
+ recipe_object.author=recipe_information['author']
+ except:
+ pass
+ finally:
+ recipe_object.save()
And also, if we put this in a try except block, maybe it is best to
print the traceback then let it go.
We might consider to use a try/except block to the get_task method.
---------
+ for bl in sorted(self.internal_state['build_layers'],
reverse=True, key=_slkey):
+ if (path.startswith(bl.layer.local_path)):
+ return bl
Should we add a comment here with what's the motivation behind this ?
--------------
+ identifier = event.taskfile + event.taskname
This also needs to change if we consider my advice on using smaller
identifier values, from above.
--------------
+ if isinstance(event, bb.runqueue.runQueueTaskCompleted):
+ task_information['outcome'] = 3 # TODO: needs to use
constants
The Django model updates from this patch states the default for this is
value 4.
Either this or that need to change for consistency reasons.
--------------
+ e = event
+ e.taskname = pn
Do we need to do this? I think event instead of e makes the code more
clear.
------------
+ task_obj = self.orm_wrapper.update_task_object(task_info)
Caution: This method doesn't return anything, so it may be better just
to call it, instead of assigning it to an attribute:
self.orm_wrapper.update_task_object(task_info)
-------------
That's about it :)
I hope this helps!
Thank you,
Calin
On 19.07.2013 01:35, Damian, Alexandru wrote:
Hi guys,
I've performed a deep refactoring of the database interface code.
This was triggered by the opportunity to use dependency information
which now we dump from Bitbake to pre-load recipe and task information
prior to actual running.
This is fairly deep, as I moved a lot of code around. Because of this,
I would like a round of review before merging in the webhob-master. As
such, the code has been pushed to:
http://git.yoctoproject.org/cgit/cgit.cgi/poky-contrib/log/?h=webhob-poky/master-next
contrib/webhob-poky/master-next
Can you please check this out and let me know if you find something
spotty ?
Cheers,
Alex
--
Alex Damian
Yocto Project
SSG / OTC
_______________________________________________
webhob mailing list
[email protected]
https://lists.yoctoproject.org/listinfo/webhob
