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

[Vered Volansky]

----- Original Message -----
> From: "David Caro" <dcaroest at redhat.com>
> To: "Dan Kenigsberg" <danken at redhat.com>
> Cc: "Vered Volansky" <vered at redhat.com>, "infra" <infra at ovirt.org>
> 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 at redhat.com>
> >>>> To: "infra" <infra at ovirt.org>
> >>>> 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_gerrit/852/consoleFull
> >>>>
> >>>> Thanks,
> >>>> Vered
> >>>> _______________________________________________
> >>>> Infra mailing list
> >>>> Infra at ovirt.org
> >>>> http://lists.ovirt.org/mailman/listinfo/infra
> >>>>
> >>> _______________________________________________
> >>> Infra mailing list
> >>> Infra at 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.
I've accepted Dan's proposal, it is indeed cleaner this way.
Changed my test accordingly.
Thank you both.
> 
> --
> David Caro
> 
> Red Hat S.L.
> Continuous Integration Engineer - EMEA ENG Virtualization R&D
> 
> Email: dcaro at redhat.com
> Web: www.redhat.com
> RHT Global #: 82-62605
> 
> 
> _______________________________________________
> Infra mailing list
> Infra at ovirt.org
> http://lists.ovirt.org/mailman/listinfo/infra
>