[chef] Re: Re: Re: Re: Re: Re: Re: Re: Re: Re: Re: Re: COOK-3084 - wrong python on RHEL5

[Alex Kiernan < >]

If I had a free hand I'd pick this one https://github.com/akiernan/python/tree/remove-interpreter as I feel that making interpreter a first class attribute sends the wrong message, but a major version bump for a single change seems excessive.

Given I don't have a free hand, my preferred option would be https://github.com/akiernan/python/tree/avoid-venv-python since it doesn't gratuitously pass in --python and feels like it has the best chance of doing what the virtualenv authors intended.

I actually like https://github.com/akiernan/python/tree/handle-rhel5 the least, but I initially did it like that in an effort to find the smallest possible change which would resolve the problem (and most likely to get accepted... oh how wrong I was :))

> My question would be regarding the test here:
> https://github.com/akiernan/python/compare/opscode-cookbooks:master...handle-rhel5#L7R29
> 
> Should/can this default to a node attribute, so if the attribute was
> modified to "/usr/bin/python27" it could pass as well? Or do we
> refrain from adding attributes to tests?

I'm always wary of tests that do that kind of thing, it feels like it's too easy to get into a situation where you're really doing `assert x == x'. But I guess these are integration tests and we're taking a trip through the runtime so that shouldn't apply; equally I really can't see anyone being crazy enough to try and get EL5 up to Python 2.7.

In all honesty I think the tests I've added actually want refactoring into a proper virtualenv test suite, but that doesn't really sit well with a red-green defect fixing style; I'd rather get this change in then think about the refactor.

On Mon, Jun 10, 2013 at 10:31 PM, Mike < " target="_blank"> > wrote:

My vote is the patch-level change on this branch:
https://github.com/akiernan/python/tree/handle-rhel5.
Compare here: https://github.com/akiernan/python/compare/opscode-cookbooks:master...handle-rhel5

My question would be regarding the test here:
https://github.com/akiernan/python/compare/opscode-cookbooks:master...handle-rhel5#L7R29

Should/can this default to a node attribute, so if the attribute was
modified to "/usr/bin/python27" it could pass as well? Or do we
refrain from adding attributes to tests?

Anyways, this is awesome work Alex - very thorough!
-M

On Mon, Jun 10, 2013 at 5:15 PM, Alex Kiernan < "> > wrote:
>
> On 10 Jun 2013 21:54, "Bryan McLellan" < "> > wrote:
>>
>> On Sun, Jun 9, 2013 at 1:31 PM, Alex Kiernan < "> >
>> wrote:
>> > Just waiting on the test-suite run to complete before pushing new
>> > branches which include minitest assertions.
>>
>> Is this pushed?
>
> It is.
>
>> Are the branches all set?
>>
>
> Should be...
>
>> Do we need to vote or did logic win out here somewhere? Who still
>> needs some convincing?
>>
>
> If we can get consensus on which approach I'll feel the test run across that
> branch as it's roughly 90 minutes per test run per branch.

--
Alex Kiernan