Re: [openstack-dev] [All] LOG.warning/LOG.warn

2014-08-19 Thread Flavio Percoco
On 08/18/2014 06:06 PM, Daniel P. Berrange wrote:
 On Mon, Aug 18, 2014 at 11:27:39AM -0400, Doug Hellmann wrote:

 On Aug 18, 2014, at 10:15 AM, Daniel P. Berrange berra...@redhat.com wrote:

 On Mon, Aug 18, 2014 at 07:57:28AM +1000, Michael Still wrote:
 My recollection is that this was a request from the oslo team, but it
 was so long ago that I don't recall the details.

 I think the change is low value, so should only be done when someone
 is changing the logging in a file already (the log hinting for
 example).

 The lazy conversion approach really encourages bad practice and is very
 wasteful for developers/reviewers. In GIT commit guidelines we explicitly
 say not to make code cleanups in their code that are unrelated to the
 feature/bug being addressed. When we have done lazy conversion for this
 kind of thing, I've seen it waste a hell of alot of time for developers.
 People are never entirely clear which is the preferred style, so they
 end up just making a guess which will often be wrong. So now we consume
 scarce reviewer time pointing this out to people over  over  over again,
 and waste developer time having them re-post their patches again.

 If we want to change LOG.warning to LOG.warn, we should do a single
 patch with a global search  replace to get the pain over  done with
 as soon as possible, then enforce it with a hacking rule. No reviewer
 time gets wasted and developers will see their mistake right away.

 The only issue I can see with that approach is causing existing patches
 to have to be rebased. That can be mitigated by making these sorts of
 cleanups at a time when other feature changes aren’t going to be under
 development, though.
 
 The pessimist in me says that the majority of existing patches are going
 have to be rebased many more times for unrelated reasons already, so this
 won't be as bad as you might fear. It would make sense to avoid doing
 these kind of cleanups immediately before any release milestones though,
 since there is no sense in intentionally inflicting this pain at the
 worst possible time :-)
 
 So IMHO there is a tiny window for someone to propose this for Juno
 right now if reviewers commit to merging it quickly. Otherwise any
 conversion should wait until Kilo opens, and don't attempt to do a
 piece-by-piece conversion in the meantime.

I agree with Daniel here. I'd rather have a single, ninja-approved patch
changing all matches than several small patches that introduce unrelated
changes to replace warn with warning. An additional benefit is that the
single-replacement patch may also be used as a reference for future
commits using `warn` instead of `warning`.

My $0.02,
Flavio


-- 
@flaper87
Flavio Percoco

___
OpenStack-dev mailing list
OpenStack-dev@lists.openstack.org
http://lists.openstack.org/cgi-bin/mailman/listinfo/openstack-dev


Re: [openstack-dev] [All] LOG.warning/LOG.warn

2014-08-18 Thread Doug Hellmann
warn() and warning() are synonyms (literally the same method, aliased). We had 
to add a similar alias in the oslo ContextAdapter to support code using both 
forms in existing code. If the documented form is warning(), then I agree we 
should stick with that, although I don’t think the churn caused by making that 
change is a good idea at this point in the schedule (Michael’s heuristic below 
makes sense).

Doug

On Aug 17, 2014, at 5:57 PM, Michael Still mi...@stillhq.com wrote:

 My recollection is that this was a request from the oslo team, but it
 was so long ago that I don't recall the details.
 
 I think the change is low value, so should only be done when someone
 is changing the logging in a file already (the log hinting for
 example).
 
 Michael
 
 On Sun, Aug 17, 2014 at 4:26 PM, Gary Kotton gkot...@vmware.com wrote:
 Hi,
 Over the last few weeks I have seen a number of patches where LOG.warn is
 replacing LOG.warning. I think that if we do something it should be the
 opposite as warning is the documented one in python 2 and 3
 https://docs.python.org/3/howto/logging.html.
 Any thoughts?
 Thanks
 Gary
 
 ___
 OpenStack-dev mailing list
 OpenStack-dev@lists.openstack.org
 http://lists.openstack.org/cgi-bin/mailman/listinfo/openstack-dev
 
 
 
 
 -- 
 Rackspace Australia
 
 ___
 OpenStack-dev mailing list
 OpenStack-dev@lists.openstack.org
 http://lists.openstack.org/cgi-bin/mailman/listinfo/openstack-dev


___
OpenStack-dev mailing list
OpenStack-dev@lists.openstack.org
http://lists.openstack.org/cgi-bin/mailman/listinfo/openstack-dev


Re: [openstack-dev] [All] LOG.warning/LOG.warn

2014-08-18 Thread Daniel P. Berrange
On Mon, Aug 18, 2014 at 07:57:28AM +1000, Michael Still wrote:
 My recollection is that this was a request from the oslo team, but it
 was so long ago that I don't recall the details.
 
 I think the change is low value, so should only be done when someone
 is changing the logging in a file already (the log hinting for
 example).

The lazy conversion approach really encourages bad practice and is very
wasteful for developers/reviewers. In GIT commit guidelines we explicitly
say not to make code cleanups in their code that are unrelated to the
feature/bug being addressed. When we have done lazy conversion for this
kind of thing, I've seen it waste a hell of alot of time for developers.
People are never entirely clear which is the preferred style, so they
end up just making a guess which will often be wrong. So now we consume
scarce reviewer time pointing this out to people over  over  over again,
and waste developer time having them re-post their patches again.

If we want to change LOG.warning to LOG.warn, we should do a single
patch with a global search  replace to get the pain over  done with
as soon as possible, then enforce it with a hacking rule. No reviewer
time gets wasted and developers will see their mistake right away.

Regards,
Daniel
-- 
|: http://berrange.com  -o-http://www.flickr.com/photos/dberrange/ :|
|: http://libvirt.org  -o- http://virt-manager.org :|
|: http://autobuild.org   -o- http://search.cpan.org/~danberr/ :|
|: http://entangle-photo.org   -o-   http://live.gnome.org/gtk-vnc :|

___
OpenStack-dev mailing list
OpenStack-dev@lists.openstack.org
http://lists.openstack.org/cgi-bin/mailman/listinfo/openstack-dev


Re: [openstack-dev] [All] LOG.warning/LOG.warn

2014-08-18 Thread Doug Hellmann

On Aug 18, 2014, at 10:15 AM, Daniel P. Berrange berra...@redhat.com wrote:

 On Mon, Aug 18, 2014 at 07:57:28AM +1000, Michael Still wrote:
 My recollection is that this was a request from the oslo team, but it
 was so long ago that I don't recall the details.
 
 I think the change is low value, so should only be done when someone
 is changing the logging in a file already (the log hinting for
 example).
 
 The lazy conversion approach really encourages bad practice and is very
 wasteful for developers/reviewers. In GIT commit guidelines we explicitly
 say not to make code cleanups in their code that are unrelated to the
 feature/bug being addressed. When we have done lazy conversion for this
 kind of thing, I've seen it waste a hell of alot of time for developers.
 People are never entirely clear which is the preferred style, so they
 end up just making a guess which will often be wrong. So now we consume
 scarce reviewer time pointing this out to people over  over  over again,
 and waste developer time having them re-post their patches again.
 
 If we want to change LOG.warning to LOG.warn, we should do a single
 patch with a global search  replace to get the pain over  done with
 as soon as possible, then enforce it with a hacking rule. No reviewer
 time gets wasted and developers will see their mistake right away.
 
 Regards,
 Daniel

The only issue I can see with that approach is causing existing patches to have 
to be rebased. That can be mitigated by making these sorts of cleanups at a 
time when other feature changes aren’t going to be under development, though.

Doug

 -- 
 |: http://berrange.com  -o-http://www.flickr.com/photos/dberrange/ :|
 |: http://libvirt.org  -o- http://virt-manager.org :|
 |: http://autobuild.org   -o- http://search.cpan.org/~danberr/ :|
 |: http://entangle-photo.org   -o-   http://live.gnome.org/gtk-vnc :|
 
 ___
 OpenStack-dev mailing list
 OpenStack-dev@lists.openstack.org
 http://lists.openstack.org/cgi-bin/mailman/listinfo/openstack-dev


___
OpenStack-dev mailing list
OpenStack-dev@lists.openstack.org
http://lists.openstack.org/cgi-bin/mailman/listinfo/openstack-dev


Re: [openstack-dev] [All] LOG.warning/LOG.warn

2014-08-18 Thread Daniel P. Berrange
On Mon, Aug 18, 2014 at 11:27:39AM -0400, Doug Hellmann wrote:
 
 On Aug 18, 2014, at 10:15 AM, Daniel P. Berrange berra...@redhat.com wrote:
 
  On Mon, Aug 18, 2014 at 07:57:28AM +1000, Michael Still wrote:
  My recollection is that this was a request from the oslo team, but it
  was so long ago that I don't recall the details.
  
  I think the change is low value, so should only be done when someone
  is changing the logging in a file already (the log hinting for
  example).
  
  The lazy conversion approach really encourages bad practice and is very
  wasteful for developers/reviewers. In GIT commit guidelines we explicitly
  say not to make code cleanups in their code that are unrelated to the
  feature/bug being addressed. When we have done lazy conversion for this
  kind of thing, I've seen it waste a hell of alot of time for developers.
  People are never entirely clear which is the preferred style, so they
  end up just making a guess which will often be wrong. So now we consume
  scarce reviewer time pointing this out to people over  over  over again,
  and waste developer time having them re-post their patches again.
  
  If we want to change LOG.warning to LOG.warn, we should do a single
  patch with a global search  replace to get the pain over  done with
  as soon as possible, then enforce it with a hacking rule. No reviewer
  time gets wasted and developers will see their mistake right away.
 
 The only issue I can see with that approach is causing existing patches
 to have to be rebased. That can be mitigated by making these sorts of
 cleanups at a time when other feature changes aren’t going to be under
 development, though.

The pessimist in me says that the majority of existing patches are going
have to be rebased many more times for unrelated reasons already, so this
won't be as bad as you might fear. It would make sense to avoid doing
these kind of cleanups immediately before any release milestones though,
since there is no sense in intentionally inflicting this pain at the
worst possible time :-)

So IMHO there is a tiny window for someone to propose this for Juno
right now if reviewers commit to merging it quickly. Otherwise any
conversion should wait until Kilo opens, and don't attempt to do a
piece-by-piece conversion in the meantime.

Regards,
Daniel
-- 
|: http://berrange.com  -o-http://www.flickr.com/photos/dberrange/ :|
|: http://libvirt.org  -o- http://virt-manager.org :|
|: http://autobuild.org   -o- http://search.cpan.org/~danberr/ :|
|: http://entangle-photo.org   -o-   http://live.gnome.org/gtk-vnc :|

___
OpenStack-dev mailing list
OpenStack-dev@lists.openstack.org
http://lists.openstack.org/cgi-bin/mailman/listinfo/openstack-dev


Re: [openstack-dev] [All] LOG.warning/LOG.warn

2014-08-18 Thread Joe Gordon
On Sun, Aug 17, 2014 at 9:24 AM, Jay Bryant jsbry...@electronicjungle.net
wrote:

 +2

 I prefer the LOG.warning format and support that given the documentation
 you shared.

 If there is agreement I would create a hacking check.


I think a better approach is to just not care which i used here. IMHO
mixing up LOG.warn and LOG.warning doesn't hurt readability in any
meaningful way. And not caring will result in less churn for very little
benefit.


  Jay
 On Aug 17, 2014 1:28 AM, Gary Kotton gkot...@vmware.com wrote:

  Hi,
 Over the last few weeks I have seen a number of patches where LOG.warn is
 replacing LOG.warning. I think that if we do something it should be the
 opposite as warning is the documented one in python 2 and 3
 https://docs.python.org/3/howto/logging.html.
 Any thoughts?
 Thanks
 Gary

 ___
 OpenStack-dev mailing list
 OpenStack-dev@lists.openstack.org
 http://lists.openstack.org/cgi-bin/mailman/listinfo/openstack-dev


 ___
 OpenStack-dev mailing list
 OpenStack-dev@lists.openstack.org
 http://lists.openstack.org/cgi-bin/mailman/listinfo/openstack-dev


___
OpenStack-dev mailing list
OpenStack-dev@lists.openstack.org
http://lists.openstack.org/cgi-bin/mailman/listinfo/openstack-dev


Re: [openstack-dev] [All] LOG.warning/LOG.warn

2014-08-17 Thread Jay Bryant
+2

I prefer the LOG.warning format and support that given the documentation
you shared.

If there is agreement I would create a hacking check.

Jay
On Aug 17, 2014 1:28 AM, Gary Kotton gkot...@vmware.com wrote:

  Hi,
 Over the last few weeks I have seen a number of patches where LOG.warn is
 replacing LOG.warning. I think that if we do something it should be the
 opposite as warning is the documented one in python 2 and 3
 https://docs.python.org/3/howto/logging.html.
 Any thoughts?
 Thanks
 Gary

 ___
 OpenStack-dev mailing list
 OpenStack-dev@lists.openstack.org
 http://lists.openstack.org/cgi-bin/mailman/listinfo/openstack-dev


___
OpenStack-dev mailing list
OpenStack-dev@lists.openstack.org
http://lists.openstack.org/cgi-bin/mailman/listinfo/openstack-dev


Re: [openstack-dev] [All] LOG.warning/LOG.warn

2014-08-17 Thread Michael Still
My recollection is that this was a request from the oslo team, but it
was so long ago that I don't recall the details.

I think the change is low value, so should only be done when someone
is changing the logging in a file already (the log hinting for
example).

Michael

On Sun, Aug 17, 2014 at 4:26 PM, Gary Kotton gkot...@vmware.com wrote:
 Hi,
 Over the last few weeks I have seen a number of patches where LOG.warn is
 replacing LOG.warning. I think that if we do something it should be the
 opposite as warning is the documented one in python 2 and 3
 https://docs.python.org/3/howto/logging.html.
 Any thoughts?
 Thanks
 Gary

 ___
 OpenStack-dev mailing list
 OpenStack-dev@lists.openstack.org
 http://lists.openstack.org/cgi-bin/mailman/listinfo/openstack-dev




-- 
Rackspace Australia

___
OpenStack-dev mailing list
OpenStack-dev@lists.openstack.org
http://lists.openstack.org/cgi-bin/mailman/listinfo/openstack-dev