Merge lp://staging/~fgiff/linaro-android-bot-review/update-gerrit-and-lava-integration-2 into lp://staging/linaro-android-bot-review

I think "logger" is a bad name for what it does, it should be IntegrationInfoStore or something.

It's unclear if default_env is still used. Or rather, it seems to be not used, but why running testgerritconnection.py fails then?

$ python testgerritconnection.py
.E.Error: Failed to load known_hosts file so unable to connect to gerrit.
EE
======================================================================
ERROR: testGet_starred_list (__main__.GerritConnectionTestCase)
----------------------------------------------------------------------
Traceback (most recent call last):
  File "testgerritconnection.py", line 35, in testGet_starred_list
    self.gc.get_starred_list()[0]):
  File "/home/pfalcon/devel/linaro/update-gerrit-and-lava-integration-2/gerritconnection.py", line 59, in get_starred_list
    starredby:{}'.format(self.ssh_user))
ValueError: zero length field name in format

testgerritconnection.py, being a unit test, should not expect any external context, or if it needs it, should pre-setup it.

Looking it where it fails,

        """SSH server key list e.g. ~/.ssh/known_hosts."""
        self.review_pub_key = review_pub_key

known_hosts and pub key are two different things, naming one another is confusing.

> testgerritconnection.py, being a unit test, should not expect any external
> context, or if it needs it, should pre-setup it.

Oops. known_hosts is there, but there's still error like above.

Well, I wondered why "{}".format(foo) syntax was used at all, and now it seems it's good time to ask. Please provide good reasons why it was used, or rewrite it using classical cross-language printf-style syntax ("%s" % foo). Because that {} is not compatible, confusing, superfluous construct (see http://stackoverflow.com/questions/5446964/valueerror-zero-length-field-name-in-format-error-in-python-3-0-3-1-3-2 for incompatible part).

Cannot continue with testing - doesn't work with Python 2.x (2.6.6 here), test outputs error message when there's actually no error.

> I think "logger" is a bad name for what it does, it should be IntegrationInfoStore or something.

Sure. I called it logger as we were previously writing short logs to the filesystem. Since we now modify the records, it's not really a log any more, so a name change would help.

> testgerritconnection.py, being a unit test, should not expect any external context, or if it needs it, should pre-setup it.

I agree. If anything isn't set up, that's a bug.

> known_hosts and pub key are two different things, naming one another is confusing.

True, I'll rename it known_hosts.

> Well, I wondered why "{}".format(foo) syntax was used at all...

PEP 3101 'proposes a new system for built-in string formatting operations, intended as a replacement for the existing '%' string formatting operator.'

I don't have a particular preference for one syntax over another. This one sounded newer (less likely to get deprecated).

> Cannot continue with testing - doesn't work with Python 2.x (2.6.6 here), test outputs error message when there's actually no error.

I'll see what I can do about fixing that. It runs on my system, (python 2.7.1) so maybe there's a dependency on something in my environment.

>> Cannot continue with testing - doesn't work with Python 2.x (2.6.6 here), test outputs error message when there's actually no error.

>I'll see what I can do about fixing that. It runs on my system, (python 2.7.1) so maybe there's a dependency on something in my environment.

So apparently in Python >= 2.7 "{}".format() is fine, but in Python <= 2.6 this is an error. I've suppressed non-test output when testing, so this should be fixed, but we require Python >= 2.7 to run.

Did some renaming and disabled building manifest changes (doesn't work in the current set-up, something about the mirroring.) See v17 changelog.

On Fri, 23 Sep 2011 07:43:23 -0000
Frans Gifford <email address hidden> wrote:

[]
>
> > Well, I wondered why "{}".format(foo) syntax was used at all...
>
> PEP 3101 'proposes a new system for built-in string formatting
> operations, intended as a replacement for the existing '%' string
> formatting operator.'
>
> I don't have a particular preference for one syntax over another.
> This one sounded newer (less likely to get deprecated).

With all due respect, existence of 4-digit numbered PEP with "intended
as a replacement for the existing" in title doesn't actually make it
replace anything existing. To replace *that* existing, one need to
evaporate all previous versions of Python, and rewrite history of IT
altogether, removing printf() syntax from the most used language in
industry. Not even guy with acm.org in his email (author of those fine
bell lettres) can do that.

> So apparently in Python >= 2.7 "{}".format() is fine, but in Python
> <= 2.6 this is an error. I've suppressed non-test output when
> testing, so this should be fixed, but we require Python >= 2.7 to
> run.

But we run Python 2.6 on our servers. And no, I don't think it's worth
doing upgrade (procrastination and risk management issues). So, that
needs to be fixed. And that exactly shows what problems I have with that
PEP and consequently its syntax. Now we need to spend extra effort on
fixing that, plus me smells in that PEP a Perl conspiracy wanting to
ruin Python bliss of doing simple things ONE way, so me screams of
fighting back and removing that syntax altogether. Nothing of that would
happen if there were no that PEP. So, only things it brings are
confusion and strife. (Which is a recursive reason to saying no to it.)

--
Best Regards,
Paul

Linaro.org | Open source software for ARM SoCs
Follow Linaro: http://www.facebook.com/pages/Linaro
http://twitter.com/#!/linaroorg - http://www.linaro.org/linaro-blog

Hi Paul,

I was explaining where the syntax came from, not defending it ;-)

Since it breaks the script for Python 2.6, which is still actively
maintained (until Oct 2013), it's worth fixing, so I'll make sure it
gets changed today.

Regards,

Frans

On 23 September 2011 13:22, Paul Sokolovsky <email address hidden> wrote:
> On Fri, 23 Sep 2011 07:43:23 -0000
> Frans Gifford <email address hidden> wrote:
>
> []
>>
>> > Well, I wondered why "{}".format(foo) syntax was used at all...
>>
>> PEP 3101 'proposes a new system for built-in string formatting
>> operations, intended as a replacement for the existing '%' string
>> formatting operator.'
>>
>> I don't have a particular preference for one syntax over another.
>> This one sounded newer (less likely to get deprecated).
>
> With all due respect, existence of 4-digit numbered PEP with "intended
> as a replacement for the existing" in title doesn't actually make it
> replace anything existing. To replace *that* existing, one need to
> evaporate all previous versions of Python, and rewrite history of IT
> altogether, removing printf() syntax from the most used language in
> industry. Not even guy with acm.org in his email (author of those fine
> bell lettres) can do that.
>
>> So apparently in Python >= 2.7 "{}".format() is fine, but in Python
>> <= 2.6 this is an error. I've suppressed non-test output when
>> testing, so this should be fixed, but we require Python >= 2.7 to
>> run.
>
> But we run Python 2.6 on our servers. And no, I don't think it's worth
> doing upgrade (procrastination and risk management issues). So, that
> needs to be fixed. And that exactly shows what problems I have with that
> PEP and consequently its syntax. Now we need to spend extra effort on
> fixing that, plus me smells in that PEP a Perl conspiracy wanting to
> ruin Python bliss of doing simple things ONE way, so me screams of
> fighting back and removing that syntax altogether. Nothing of that would
> happen if there were no that PEP. So, only things it brings are
> confusion and strife. (Which is a recursive reason to saying no to it.)
>
>
> --
> Best Regards,
> Paul
>
> Linaro.org | Open source software for ARM SoCs
> Follow Linaro: http://www.facebook.com/pages/Linaro
> http://twitter.com/#!/linaroorg - http://www.linaro.org/linaro-blog
>
> https://code.launchpad.net/~fgiff/linaro-android-bot-review/update-gerrit-and-lava-integration-2/+merge/76172
> You are the owner of lp:~fgiff/linaro-android-bot-review/update-gerrit-and-lava-integration-2.
>

--
Linaro.org │ Open source software for ARM SoCs

Follow Linaro: Facebook | Twitter | Blog

On Fri, 23 Sep 2011 12:37:32 -0000
Frans Gifford <email address hidden> wrote:

> Hi Paul,
>
> I was explaining where the syntax came from, not defending it ;-)

Me too, explaining why my preference would be to switch to "%s" % foo,
instead of just stuffing 1's & 2's between {}, and see if you still
insist that format() is kept ;-).

>
> Since it breaks the script for Python 2.6, which is still actively
> maintained (until Oct 2013), it's worth fixing, so I'll make sure it
> gets changed today.

Please don't - I know that doing such changes are rather boring, so I'd
like to do it myself instead, that will also give me chance to review
the code more thoroughly.

--
Best Regards,
Paul

Linaro.org | Open source software for ARM SoCs
Follow Linaro: http://www.facebook.com/pages/Linaro
http://twitter.com/#!/linaroorg - http://www.linaro.org/linaro-blog

On 23 September 2011 14:04, Paul Sokolovsky <email address hidden> wrote:
> On Fri, 23 Sep 2011 12:37:32 -0000
> Frans Gifford <email address hidden> wrote:
>
>> Hi Paul,
>>
>> I was explaining where the syntax came from, not defending it ;-)
>
> Me too, explaining why my preference would be to switch to "%s" % foo,
> instead of just stuffing 1's & 2's between {}, and see if you still
> insist that format() is kept ;-).
>
>>
>> Since it breaks the script for Python 2.6, which is still actively
>> maintained (until Oct 2013), it's worth fixing, so I'll make sure it
>> gets changed today.
>
> Please don't - I know that doing such changes are rather boring, so I'd
> like to do it myself instead, that will also give me chance to review
> the code more thoroughly.
>
Whoops, too late: I already did, and I ran it through python2.6 just
to be sure it worked.

Regards,

Frans
>
> --
> Best Regards,
> Paul
>
> Linaro.org | Open source software for ARM SoCs
> Follow Linaro: http://www.facebook.com/pages/Linaro
> http://twitter.com/#!/linaroorg - http://www.linaro.org/linaro-blog
>
> https://code.launchpad.net/~fgiff/linaro-android-bot-review/update-gerrit-and-lava-integration-2/+merge/76172
> You are the owner of lp:~fgiff/linaro-android-bot-review/update-gerrit-and-lava-integration-2.
>

--
Linaro.org │ Open source software for ARM SoCs

Follow Linaro: Facebook | Twitter | Blog