Add hacking check for deprecated library function os.popen().
This hacking prevents new os.popen() in the code.
Closes-Bug: 1529836
Change-Id: I09ad101861b790d2f9bec45757b8c921e68b696e
Adds a hacking check if a config option provides enough help text.
To do so, the check counts the length of the help text. It uses
a low number in order to avoid a break. As soon as we have agreed
on a higher standard and changed the options accordingly, we can
increase that number.
Change-Id: If31339c428953c6a7bf721ff92e5999e8cb5b569
This also adds a check to nova/hacking/checks.py that
should check dict.iteritems, dict.itervalues and
dict.iterkeys should not be used in the future.
NOTE:
In _dict_from_object() method of test_db_api.py items() method
is called on dict and iteritems() method is called if object is
other than dict. Changed it to directly use obj.items as obj can
either be dict or object of nova.db.sqlalchemy.models.<Class> and
both have items() method.
Partially-Implements: blueprint nova-python3-mitaka
Change-Id: Ib0fe3066199b92e4f013bb15312ada7515fa3d7b
This adds a hackign check which raises a pep8 error if a config
option was found in location where it shouldn't be.
Change-Id: I75df3404cc6178aa179e96d75f154e389c3008af
Removed use of contextlib.nested call from codebase as
contextlib.nested is not compatible with Python 3.
Added hacking check to catch if any new instances are added
to the codebase.
Change-Id: Ib78102bc013e4cc91ba54d79aa2815f4cf9f446d
Change Id52c30bb5ded2184d772e6026b0f04f9a0efeb55 added a hacking check
for greenthread.spawn(). Since eventlet.spawn() calls
greenthread.spawn() under the covers, it should also be checked. Because
there are still occurrences of eventlet.spawn(), these were also cleaned
up in order to pass the added hacking check.
Co-Authored-By: Qin Zhao <chaochin@gmail.com>
Change-Id: Ia125b4ad5e84bf48091af5a7a483b89629f0ec31
Related-Bug: #1404268
Closes-Bug: #1468513
Because greenthread.spawn() and spawn_n() are missing a nova context
(see I3623e60c49e442e2431cf017540422aa59bc285a and
Ia5175c9729069df3d779237ba6039cf5bc920018), nova.utils.spawn() and
spawn_n() should be used when spawning threads. This change adds a
hacking check to assert this is being done during pep8.
Change-Id: Id52c30bb5ded2184d772e6026b0f04f9a0efeb55
Related-Bug: #1404268
Closes-Bug: #1468513
Latest oslo libraries do not support the old oslo
namespace based imports, so the import check in our hacking
rules are redundant as CI jobs will fail for sure if someone
tries to use oslo namespace based imports.
Change-Id: I49c74ade374582f53a3678a1bc7df194c24e892e
The sphinx latex generation generates invalid latex that won't compile
when you try to use it.[1][2] This commit adds a helper script to generate
the sphinx latex and then modify it so it'll work. It depends on
ImageMagick convert and sed being available to work.
[1] https://github.com/sphinx-doc/sphinx/issues/1907
[2] https://github.com/sphinx-doc/sphinx/issues/1959
Change-Id: Id289c10907aaddae2483f18b39063852ec699d66
There are three updates here:
1. Point to the correct path of development.environment.rst.
2. Update the path to test_wsgi.py after the test restructure
that happened in Kilo.
3. Just tell people to use `tox -e docs` for building docs.
Change-Id: I03295a6d9c90e9a2962999726d254bc4971c4909
There is raise_feature_not_supported() for returning a HTTP501
response with consistent error message, and this patch adds a rule
for enforcing to use the method on v2.1 API.
Partially implements blueprint v2-on-v3-api
Change-Id: I06f254fd9c8d8b6aac4ed135c6c407f3a993431f
8510d3aaeeb18bdbe333d2d5d4c335f3732c4848 removed N331
checking, so now we need treat LOG.warn and LOG.warning
same, so this patch adds the same checking to LOG.warn.
Change-Id: Ifb3addfe116f8f7abb8750826b0217dfbd766439
As part of this commit 89cd6a0c493e26b5a9e017c99d731464292abbaf ("move
all tests to nova/tests/unit") README.rst was moved from nova/tests/ to
nova/tests/unit/. Update HACKING.rst to reflect that.
Change-Id: I282baac560d6035e453542812c36b505ddc07bc1
The following replacements were done in tests to have
clearer messages in case of failure:
- assertEqual(a in b, True) with assertIn(a, b)
- assertEqual(a in b, False) with assertNotIn(a, b)
The error message would now be like:
'abc' not in ['a', 'b', 'c']
rather than:
MismatchError: False != True
Change-Id: I514daca3a470feef5d332a1a319cba15256fc6ea
Exception lines in unit tests won't ever be run in production, no reason to
translate them.
Added hacking rule for not importing i18n translation in tests.
Change-Id: I92f546166d4e0b2fa8dc2018c6d3e268b8ec4c0b
PEP-0274 introduced dict comprehensions to replace dict constructor
with a sequence of length-2 sequences, these are benefits copied
from [1]:
The dictionary constructor approach has two distinct disadvantages
from the proposed syntax though. First, it isn't as legible as a
dict comprehension. Second, it forces the programmer to create an
in-core list object first, which could be expensive.
Nova dropped python 2.6 support, we can leverage this now.
There is deep dive about PEP-0274[2] and basic tests about
performance[3].
Note: This commit doesn't handle dict constructor with kwagrs.
This commit also adds a hacking rule.
[1]http://legacy.python.org/dev/peps/pep-0274/
[2]http://doughellmann.com/2012/11/12/the-performance-impact-of-using-dict-instead-of-in-cpython-2-7-2.html
[3]http://paste.openstack.org/show/154798/
Change-Id: Ifb5cb05b9cc2b8758d5a8e34f7792470a73d7c40
The unit test log ends up with DeprecationWarning(s) from the outdated
calls to assertRaisesRegexp. We should switch to using assertRaisesRegex
instead. This commit eliminates these warnings from the log and the hacking
rule N344 ensures that folks don't end up adding fresh code down the line
with the outdated assertRaisesRegexp as well
Partial-Bug: #1407736
Change-Id: Ifba672f7568d5159c63bf88c534812e4e3a26d5a
The following replacements were done in tests to have
clearer messages in case of failure:
- assertTrue(a in b) with assertIn(a, b)
- assertTrue(a not in b) with assertNotIn(a, b)
- assertFalse(a in b) with assertNotIn(a, b)
The error message would now be like:
'abc' not in ['a', 'b', 'c']
rather than:
'False is not True'.
Change-Id: I92d039731f17b731d302c35fb619300078b8a638
Ib63da2d845843410634a1df0261af33b973daf32 is an example where
new code had to be fixed for oslo_concurrency imports. Let us
add a hacking check to prevent such regression. When we update
to new oslo libraries with non-namespace'd imports, we should
update the regexp in this hacking rule
Change-Id: I44536d477d06ddc1205b824bcb888b666405dce3
Because of the implementation of this decorator and the controller's metaclass,
the api_version decorator must be the outermost (ie first) decorator wherever
it is used. This patch adds a hacking check to ensure that this is the case.
This decorator is intended to be used on multiple methods with the same name
which offends F811, so '#noqa' is needed too. This will be fixed in a separate
patch.
Bad:
@some_decorator # noqa <-- '# noqa' to avoid F811
@api_version("2.5") <-- Error, needs to be the first decorator
def my_api_method...
Good:
@api_version("2.5") # noqa <-- this line still needs '# noqa' for F811
@some_decorator
def my_api_method...
Change-Id: I579c0061f03d788c477c5424d4d00ec7a6e721e1
oslo.i18n uses different marker functions to separate the
translatable messages into different catalogs, which the translation
teams can prioritize translating. For details, please refer to:
http://docs.openstack.org/developer/oslo.i18n/guidelines.html#guidelines-for-use-in-openstack
There were not marker fuctions some places in directory network.
This commit makes changes:
* Add missing marker functions
* Use ',' instead of '%' while adding variables to log messages
Added a hacking rule for the warning about checking
translation for it and checking logging level `warning` instead
alias `warn`.
Change-Id: I2bced49dc5a0408a94d5d20d85b20c682886edbe
oslo.i18n uses different marker functions to separate the
translatable messages into different catalogs, which the translation
teams can prioritize translating. For details, please refer to:
http://docs.openstack.org/developer/oslo.i18n/guidelines.html#guidelines-for-use-in-openstack
There were not marker fuctions some places in directory network.
This commit makes changes:
* Add missing marker functions
* Use ',' instead of '%' while adding variables to log messages
Added a hacking rule for the log exception about checking
translation for it.
Change-Id: If80ea6f177bb65afcdffce71550bb38fedcc54eb
oslo.i18n uses different marker functions to separate the
translatable messages into different catalogs, which the translation
teams can prioritize translating. For details, please refer to:
http://docs.openstack.org/developer/oslo.i18n/guidelines.html#guidelines-for-use-in-openstack
There were not marker fuctions some places in directory network.
This commit makes changes:
* Add missing marker functions
* Use ',' instead of '%' while adding variables to log messages
Added a hacking rule for the log info about checking
translation for it.
Change-Id: I96766d723b01082339876ed94bbaa77783322b8c
I haven't done this in a while, and I went through a number of false
starts forgetting the "--", trying to specify "filename:TestClass"
etc. before I got it to work. Hopefully this saves someone else some
time.
Change-Id: I94590009157cce8d42002089304c66c164bcd6ec
Casting exceptions to unicode using unicode() can interfere
with the proper translation of the exception message. This
is especially true when lazy translation is enabled, since it
causes the exception message to be translated immediately using
the default locale.
This is the same problem caused by using str on exceptions,
which was fixed by https://review.openstack.org/#/c/116054/
In addition to fixing these cases, this patch updates
hacking check N325, which checks for the use of str() on
exceptions, to also check for the use of unicode().
In order to make it clear which cases that cannot be caught
by the hacking check have been inspected, they have been
converted to using six.text_type() instead of unicode().
Closes-bug: #1380806
Change-Id: I87bb94fa76458e028beba28d092320057b53f70a
mock.assert_called_once() is a no-op that tests nothing. Instead
with mock.assert_called_once_with() should be used (or use
assertEqual(1, mock_obj.call_count) if you don't want to check
parameters).
Add a new HACKING rule for nova to prevent assert_called_once()
usage from creeping in.
Closes-Bug: #1365751
Change-Id: I1055093a1c31792b6411a3cd46c80b8bcaaf78c1
Replace common oslo code nova.openstack.common.db by usage
of oslo.db library and remove common code.
Replaced catching of raw sqlalchemy exceptions by catches
of oslo.db exceptions(such as DBError, DBDuplicateEntry, etc).
Co-Authored-By: Eugeniya Kudryashova <ekudryashova@mirantis.com>
Closes-Bug: #1364986
Closes-Bug: #1353131
Closes-Bug: #1283987
Closes-Bug: #1274523
Change-Id: I0649539e071b2318ec85ed5d70259c949408e64b
Translated messages should not be expanded by concatenation. Instead
the additional text should be replacement text or part of the translated
message to allow the translators as much information as possible when
doing the translations.
Also, when lazy translation is enabled, the object returned does not
support concatenation and raises an exception. Thus this patch is
needed to support blueprint: i18n-enablement.
This patch includes a hacking check for concatenation with a translated
message.
Change-Id: I8b368d275faa14b4750735445321874ce1da37d5
Partially-Implements: blueprint i18n-enablement
An exception's message can be a translatable message. If it is, the message
can contain unicode characters which will cause str to fail.
In cases where the message is explicity needed, the use of str is replaced
with the use of six.text_type. When an exception is used as a replacement
string in a format string, the logger correctly handles it without the
need for str, so it is removed.
In addition to the case where a translatable message contains unicode,
enabling lazy translation in the oslo.i18n library causes translatable
messages to be replaced with an object that does not support str, this
causes all calls to str on a translatable message to fail. Thus this patch
is also needed to support blueprint: i18n-enablement.
This patch includes a hacking check for use of str() on exceptions identified
in except statements.
Change-Id: Idb426d7f710ea69b835f70d0e883e93e9b9111d2
Partially-Implements: blueprint i18n-enablement
Commit 243879f5c51fc45f03491bcb78765945ddf76be8 added in a new hacking
check that used an existing number.
The new number is 324 (and not 323)
Change-Id: I7e604a408387438105c435ad16a1fa3d6491b642
Closes-bug: #1356815
To ensure the right message catalog is used when translating
messages we need to make sure to explicitly import '_' in
any files that use that function. We cannot count on
unit test to catch cases where the user has forgotten to
import the _() function.
This hacking check ensures that the function has been imported
anywhere that it is used. Unit tests for the hacking check are
included.
Change-Id: I9d8101916bcb449345d3123617c2ac75776d053e
Passing mutable objects as default args is a known Python pitfall.
We'd better avoid this. This commit changes mutable default args with
None, then use 'arg = arg or {}', 'arg = arg or []'. For unit code which
doesn't use the args , just set with None. This commit also adds hacking
check.
Closes-Bug: #1327473
Change-Id: I5a8492bf8ffef8e000b13b6bdfaef1968b96f816
Update a number of files to add missing translation support.
The patch adds a new hacking check - N321. This ensures that
all log messages, except debug ones, have translations.
A '# noqa' indicates that the validation will not be done on
the specific log message. This should be used in cases where
the translations do not need to be done, for example, the log
message is logging raw data.
Closes-bug: #1290261
Change-Id: Ib2ca0bfaaf432e15448c96619682c2cfd073fbbc
Commit 9235ada8c2c250603dc5b299cc08bb7a982d4fc6 did not add in
the updated hacking rule.
Change-Id: If2daf6d2e7008c36d0aba6270ac034522dcb2e2b
Closes-bug: #1325812
I've showed these docs to more people than I care to. It would be
extremely helpful to reference them in HACKING, which is generally
the first thing people read if they intend to hack.
Change-Id: I9194d98f5525e29711b4a1b26414a50b8ceba525
Commit ac8bce63f8a7ec8a2ebb214ea7f86ee4f8adecae added hacking rules
N319. This was not documented in the file HACKING.rst.
Change-Id: Ibf7917aaada88db5afe1387859a387481ec05118
Closes-bug: #1313322
Replace assertEqual(None, *) with assertIsNone in tests to have
more clear messages in case of failure.
Change-Id: I0d38a82e78fbe94657ab9a71c08422007843d179
