#238 bodhi_comment_directive crashes when update is modified during task execution
Closed: Fixed None Opened 8 years ago by zbyszek.

Description of problem:
taskotron - 2015-05-05 05:41:35 (karma: 0)
Taskotron: depcheck test ABORTED on x86_64. Result log:
https://taskotron.fedoraproject.org/taskmaster//builders/x86_64/builds/67165/steps/runtask/logs/stdio
(results are informative only)

From the above link:

[libtaskotron:bodhi_comment_directive.py:483] 2015-05-05 05:37:41 ERROR   'NoneType' object has no attribute '__getitem__'
Traceback (most recent call last):
  File "/usr/lib/python2.7/site-packages/libtaskotron/directives/bodhi_comment_directive.py", line 450, in _post_testresult
    arch)
  File "/usr/lib/python2.7/site-packages/libtaskotron/directives/bodhi_comment_directive.py", line 353, in _already_commented
    taskotron_comments = [comment for comment in update['comments']
TypeError: 'NoneType' object has no attribute '__getitem__'
[libtaskotron:bodhi_comment_directive.py:164] 2015-05-05 05:37:41 WARNING Failed to post Bodhi Comment: `netcdf-fortran-4.4.1-3.fc22,elpa-2015.02.002-2.fc22,scalapack-2.0.2-7.fc22,mpich-3.1.4-2.fc22,hdf5-1.8.14-3.fc22,mpi4py-1.3.1-6.fc22,scorep-1.3-4.fc22,paraview-4.3.1-4.fc22.1,netcdf-4.3.3.1-1.fc22,Ray-2.3.1-7.fc22,nwchem-6.5.26243-15.fc22.1,freefem++-3.31-1.4.fc22,mathgl-2.3-4.fc22,cp2k-2.6.0-2.fc22,gmsh-2.9.3-2.fc22,netgen-mesher-5.3.1-4.fc22,boost-1.57.0-6.fc22,dl_poly-1.9.20140324-8.fc22,gromacs-4.6.5-6.fc22,scotch-6.0.4-2.fc22,pypar-2.1.5_108-7.fc22,orsa-0.7.0-30.fc22,espresso-3.3.0-3.fc22,elk-3.0.18-10.fc22,ga-5.3b-16.fc22,gpaw-0.10.0.11364-10.fc22` `depcheck` `PASSED`

I don't know if the traceback is the cause of failure, but it seems possible.

How reproducible:
See https://admin.fedoraproject.org/updates/FEDORA-2015-4630/mpich-3.1.4-2.fc22. It seems to fail after some edits of the update.


This ticket had assigned some Differential requests:
D368

Interestingly enough, it sounds like this will be fixed by a patch that I'm going to be pushing to dev and stg today for testing: D362

Wasn't the purpose of that patch but I'll take fixing extra bugs with little additional effort :)

Thanks for reporting this, @zbyszek. I've already spent some time today looking into this, and this seems to be just a cosmetic issue. The depcheck run was supposed to end as ABORTED, because the update was modified during execution. It even says Update was incomplete., which is correct (although not very visible in that wall of text):

not ok - depcheck for Bodhi update mpich-3.1.4-2.fc22,elpa-2015.02.002-2.fc22,scalapack-2.0.2-7.fc22,hdf5-1.8.14-3.fc22,mpi4py-1.3.1-6.fc22,scorep-1.3-4.fc22,paraview-4.3.1-4.fc22.1,netcdf-4.3.3.1-1.fc22,netcdf-fortran-4.4.1-3.fc22,netcdf-cxx4-4.2.1-5.fc22,Ray-2.3.1-7.fc22,nwchem-6.5.26243-15.fc22.1,freefem++-3.31-1.4.fc22,mathgl-2.3-4.fc22,cp2k-2.6.0-2.fc22,gmsh-2.9.3-2.fc22,netgen-mesher-5.3.1-4.fc22,boost-1.57.0-6.fc22,dl_poly-1.9.20140324-8.fc22,gromacs-4.6.5-6.fc22,scotch-6.0.4-2.fc22,pypar-2.1.5_108-7.fc22,orsa-0.7.0-30.fc22,espresso-3.3.0-3.fc22,elk-3.0.18-10.fc22,ga-5.3b-16.fc22,gpaw-0.10.0.11364-10.fc22 # FAIL 
  ---
  arch: x86_64
  details:
    output: Update was incomplete.
  item: mpich-3.1.4-2.fc22,elpa-2015.02.002-2.fc22,scalapack-2.0.2-7.fc22,hdf5-1.8.14-3.fc22,mpi4py-1.3.1-6.fc22,scorep-1.3-4.fc22,paraview-4.3.1-4.fc22.1,netcdf-4.3.3.1-1.fc22,netcdf-fortran-4.4.1-3.fc22,netcdf-cxx4-4.2.1-5.fc22,Ray-2.3.1-7.fc22,nwchem-6.5.26243-15.fc22.1,freefem++-3.31-1.4.fc22,mathgl-2.3-4.fc22,cp2k-2.6.0-2.fc22,gmsh-2.9.3-2.fc22,netgen-mesher-5.3.1-4.fc22,boost-1.57.0-6.fc22,dl_poly-1.9.20140324-8.fc22,gromacs-4.6.5-6.fc22,scotch-6.0.4-2.fc22,pypar-2.1.5_108-7.fc22,orsa-0.7.0-30.fc22,espresso-3.3.0-3.fc22,elk-3.0.18-10.fc22,ga-5.3b-16.fc22,gpaw-0.10.0.11364-10.fc22
  outcome: ABORTED
  summary: mpich-3.1.4-2.fc22,elpa-2015.02.002-2.fc22,scalapack-2.0.2-7.fc22,hdf5-1.8.14-3.fc22,mpi4py-1.3.1-6.fc22,scorep-1.3-4.fc22,paraview-4.3.1-4.fc22.1,netcdf-4.3.3.1-1.fc22,netcdf-fortran-4.4.1-3.fc22,netcdf-cxx4-4.2.1-5.fc22,Ray-2.3.1-7.fc22,nwchem-6.5.26243-15.fc22.1,freefem++-3.31-1.4.fc22,mathgl-2.3-4.fc22,cp2k-2.6.0-2.fc22,gmsh-2.9.3-2.fc22,netgen-mesher-5.3.1-4.fc22,boost-1.57.0-6.fc22,dl_poly-1.9.20140324-8.fc22,gromacs-4.6.5-6.fc22,scotch-6.0.4-2.fc22,pypar-2.1.5_108-7.fc22,orsa-0.7.0-30.fc22,espresso-3.3.0-3.fc22,elk-3.0.18-10.fc22,ga-5.3b-16.fc22,gpaw-0.10.0.11364-10.fc22
    into F22 testing
  type: bodhi_update
  ...

But this shouldn't go to Bodhi, and D362 should fix that. Once that is deployed, the code in question will not be reached and so the exception won't occur. The update will get retested during next depcheck run, and only then the check result will be published in Bodhi.

The question is whether we should improve bodhi comment directive somehow. I don't think so, because a) checks are expected to check whether update has changed since startup (as depcheck and upgradepath do) and set ABORTED in that case b) the fix in D362 will work around this issue for such checks c) we want to get rid of bodhi comment directive as soon as possible.

Thoughts?

Thoughts?
I think that ABORTED is likely to be misconstrued as a problem with the package and not the check. Currently the "wall of text" of very hard to digest, and since the checks are asynchronous, the user is unlikely to connect the fact that the update was changed with the fact that the check was aborted. At least I didn't in my case. So yeah, I think the comment directive should be changed to be very explicit ("check aborted because ..."), and maybe use "SKIPPED" instead of "ABORTED".

I would not worry about the SKIPPED vs. ABORTED, really, and in this particular case, the ABORTED state represents what's really happening . Also, we'll likely add the semantics of states to the docs, sometimes, so that should be covered.

I'm not sure if I was clear, but once our patches hit production, maintainers shouldn't receive any ABORTED results into Bodhi. So it shouldn't confuse them. We might consider changing ABORTED to a different keyword in the future for certain use cases (and I'm sure we'll make such changes many times over in the future), but I agree with Josef that it is not necessary at the moment.

However, even if we don't send these results to Bodhi, they'll still be visible through ResultsDB. And Zbyszek has fair points in his comment. So I tried to improve the messaging from depcheck in D368 and D369. Does it look better?

All of D362, D368 and D369 got committed, I think that should be enough to cover this ticket, closing. The changes will be visible once we push a new version to production (we'll try to do that before F22 Final Freeze, I believe). Thanks for your report, Zbyszek.

Login to comment on this ticket.

Metadata