Re: How come same version pep8 does't work the same?

From: "David Caro" <dcaroest(a)redhat.com&gt; To: "Dan Kenigsberg" <danken(a)redhat.com&gt; Cc: "Vered Volansky" <vered(a)redhat.com&gt;, "infra" <infra(a)ovirt.org&gt; Sent: Friday, May 23, 2014 12:52:35 PM Subject: Re: How come same version pep8 does't work the same? On Fri 23 May 2014 11:47:52 AM CEST, Dan Kenigsberg wrote: > On Thu, May 22, 2014 at 05:45:45PM +0200, David Caro wrote: >> On Thu 22 May 2014 03:50:23 PM CEST, Yaniv Dary wrote: >>> >>> >>> ----- Original Message ----- >>>> From: "Vered Volansky" <vered(a)redhat.com&gt; >>>> To: "infra" <infra(a)ovirt.org&gt; >>>> Sent: Thursday, May 22, 2014 4:48:37 PM >>>> Subject: How come same version pep8 does't work the same? >>>> >>>> I see a patch failing to build vsdm rpm on a pep8 error. pep8 version is >>>> 1.4.6, yet the actual pep8 job works jst fine. Same pep8 version to >>>> both. >>>> How come? >>> >>> Please upgrade your version of pep8. >>> There were many changes in the latest update and this is probably causing >>> your issue. >>> >>> >>> Yaniv >>> >>>> I need to know why this happens in order to fix my job. >>>> >>>> Patch in question: http://gerrit.ovirt.org/#/c/26759/5 >>>> pep8 job: >>>> http://jenkins.ovirt.org/job/vdsm_master_pep8_gerrit/9179/console >>>> Failing job: >>>> http://jenkins.ovirt.org/job/vdsm_master_storage_functional_tests_localfs... >>>> >>>> Thanks, >>>> Vered >>>> _______________________________________________ >>>> Infra mailing list >>>> Infra(a)ovirt.org >>>> http://lists.ovirt.org/mailman/listinfo/infra >>>> >>> _______________________________________________ >>> Infra mailing list >>> Infra(a)ovirt.org >>> http://lists.ovirt.org/mailman/listinfo/infra >> >> Couple of things to have in mind here: >> >> The jobs that you sent are for different patches, I'll focus on the >> patch you sent only. >> >> Why pep8 job did pass: >> - Because the pep8 job only checks the changes made in that patch, >> and in that patch, there were no issues (git diff HEAD~) >> >> Why vdsm_master_storage_functional_tests_localfs_gerrit did not pass: >> - Because when compiling it will check all the files, not only the >> ones your patch changes, and your patch was based on an already failing >> patchset (http://gerrit.ovirt.org/#/c/27977/5/vdsm/BindingXMLRPC.py,cm). >> >> You can see in the gerrit comments for that patchset that it did >> actually fail: >> Patch Set 5: Code-Review-1 Verified-1 >> Build Unstable >> http://jenkins.ovirt.org/job/vdsm_master_pep8_gerrit/9162/ : UNSTABLE >> http://jenkins.ovirt.org/job/vdsm_master_unit_tests_gerrit/9305/ : >> SUCCESS >> http://jenkins.ovirt.org/job/vdsm_master_unit_tests_gerrit_el/8374/ : >> SUCCESS >> >> >> Mybe it's a good point to run pep8 on all the files and not only on the >> latest commit diff, to block any patchset based on a previously failing >> patch. Ideas? > > I prefer to keep the diff. The previously-failing patch should be marked > and fixed. If follow-ups are fine - let them be. > Skipping pep8 (with PEP8=true) during the storage job would have avoided > the issue in a cleaner way. Ok, let me know what you decide you want and if there are any changes needed on my side.

-- David Caro Red Hat S.L. Continuous Integration Engineer - EMEA ENG Virtualization R&D Email: dcaro(a)redhat.com Web: www.redhat.com RHT Global #: 82-62605 _______________________________________________ Infra mailing list Infra(a)ovirt.org http://lists.ovirt.org/mailman/listinfo/infra