yahoo-eng-team team mailing list archive

[Sergey Nikitin <snikitin@xxxxxxxxxxxx>]

Public bug reported:

Upper constraints for 'hacking' lib doesn't work for pep8

Couple days ago this patch was merged into Nova:
https://review.openstack.org/#/c/334048/11/nova/tests/unit/virt/libvirt/test_vif.py

It contains these lines:

   self.assertEqual(None, conf.vhost_queues)
   self.assertEqual(None, conf.driver_name)

>From pep8 point of view these lines are incorrect. Here 'assertIsNone'
should be used (rule N318)

But during merge pep8 job was 'green'. Unfortunately some number of contributors faced with fails of 'tox -e pep8' command in local repos because of these lines. It has given rise to this patch https://review.openstack.org/#/c/407870/ 
This patch fixes current problem but it doesn't fix such problems in future. 

The reason of green pep8 job was in 'hacking' lib. It was released 6 day
ago and new release 0.13.0 has a bug, wich causes our problem.

Commit with bug was already reverted from 'hacking' lib but new version
lib without bug wasn't released yet.

So the conclusion is: new release of 'hacking' lib breakes nova pep8 job. It becames false positive. 
The question is "Why new release of 'hacking' was installed?"

We have upper constraints for hacking lib in test-requirements.txt:

hacking<0.11,>=0.10.0

But we didn't use these constraints for pep8. For pep8 we only 'hacking'
dependency in tox.ini without any constraints.

https://github.com/openstack/nova/blob/master/tox.ini#L44

Because of it we install the latest version of 'hacking' each time.

I see two ways to solve it:
1) install whole test-requirements.txt for pep8. but I think it is a bad solution because we will install alot of useless dependencies.
2) add 'hacking' constraints to the tox.ini. It's not the best solution because during update of constraint in test-requirements.txt we can forget to update it in tox.ini. But now I don't see a better solution. If you have one - please share.

** Affects: nova
     Importance: Medium
         Status: New


** Tags: testing

-- 
You received this bug notification because you are a member of Yahoo!
Engineering Team, which is subscribed to OpenStack Compute (nova).
https://bugs.launchpad.net/bugs/1648054

Title:
  Upper constraints for 'hacking' lib doesn't work for pep8

Status in OpenStack Compute (nova):
  New

Bug description:
  Upper constraints for 'hacking' lib doesn't work for pep8

  Couple days ago this patch was merged into Nova:
  https://review.openstack.org/#/c/334048/11/nova/tests/unit/virt/libvirt/test_vif.py

  It contains these lines:

     self.assertEqual(None, conf.vhost_queues)
     self.assertEqual(None, conf.driver_name)

  From pep8 point of view these lines are incorrect. Here 'assertIsNone'
  should be used (rule N318)

  But during merge pep8 job was 'green'. Unfortunately some number of contributors faced with fails of 'tox -e pep8' command in local repos because of these lines. It has given rise to this patch https://review.openstack.org/#/c/407870/ 
  This patch fixes current problem but it doesn't fix such problems in future. 

  The reason of green pep8 job was in 'hacking' lib. It was released 6
  day ago and new release 0.13.0 has a bug, wich causes our problem.

  Commit with bug was already reverted from 'hacking' lib but new
  version lib without bug wasn't released yet.

  So the conclusion is: new release of 'hacking' lib breakes nova pep8 job. It becames false positive. 
  The question is "Why new release of 'hacking' was installed?"

  We have upper constraints for hacking lib in test-requirements.txt:

  hacking<0.11,>=0.10.0

  But we didn't use these constraints for pep8. For pep8 we only
  'hacking' dependency in tox.ini without any constraints.

  https://github.com/openstack/nova/blob/master/tox.ini#L44

  Because of it we install the latest version of 'hacking' each time.

  I see two ways to solve it:
  1) install whole test-requirements.txt for pep8. but I think it is a bad solution because we will install alot of useless dependencies.
  2) add 'hacking' constraints to the tox.ini. It's not the best solution because during update of constraint in test-requirements.txt we can forget to update it in tox.ini. But now I don't see a better solution. If you have one - please share.

To manage notifications about this bug go to:
https://bugs.launchpad.net/nova/+bug/1648054/+subscriptions