These block usage of methods that no longer
exist in oslo.utils 6.0.0+. Since they have
been removed, we don't need a hacking check
for this.
Change-Id: If0345c863b1750eaca3097f240fde9b976ac442e
The HACKING document had gotten out of date with a few removals and
additions of hacking checks. This gets it synced up with the current
state of our checks.
Change-Id: I8ace7662bb11d5708dd507b79f4a2476c9875ac5
Signed-off-by: Sean McGinnis <sean.mcginnis@gmail.com>
With the switch to hacking 2.0, the local checks are not enabled
anymore, update repo to use the new way of registering local checks.
Remove local vi check since hacking has this now as H106.
Change-Id: I4d03f4c7eff959017e907cac974c394af0559643
This hacking check was added back when the str()
function couldn't handle unicode characters. With
Python3 this is no longer an issue. As a result we
can now remove this check.
Change-Id: I926732062dbbd1242cd382e2593e07b4caf4ffec
This cleans up the cases where we had D001 violations so we can stop
skipping that check in doc8 runs.
Change-Id: Ie52f6ecac1a645fcbcc643b9ca63e033b622d830
Signed-off-by: Sean McGinnis <sean.mcginnis@gmail.com>
Remove the note about using mox in unit tests since
mox has been removed in cinder.
Change-Id: If7fa5f86ff5008d338f70db6dc30190f81786979
Signed-off-by: Chuck Short <chucks@redhat.com>
Starting with the Pike series, OpenStack no longer supports log
translation.
Update hacking rule to prevent log translation in all log level instead
of only debug level.
Since all log translations are invalidated, check_explicit_underscore_import
can be simplified.
Change-Id: I1105fcb7e25549017ed7fcad8f932abc1b635129
Related-Bug: #1701195
Some of the available checks are disabled by default in flake8, like:
[H106] Don’t put vim configuration in source files
[H203] Use assertIs(Not)None to check for None
This patch is to enable the H106 and H203 checks in Cinder project.
The C312 hacking rule will be removed when turn on H203.
Change-Id: I2e883c301b64d5977bbb907b63c9c144bc6f959d
This is not correct, because assertEqual(False, A) is a stricter check than assertFalse(A).
assertFalse(None) passes, but assertEqual(False, None) will fail. Therefore, this patch weakened our tests.
This reverts commit bcad8a84c0.
Change-Id: Ib7e2096c94f28d81bbf80bd2f21355a74541d9f8
This patch is to replace assertEqual(False, A) with assertFalse(A), which
the latter is more straightforward and easier to understand. Also the
relevant hacking rule is added.
Change-Id: Idd09acc6820e54c4388e183963d405b8990bc533
This is no longer needed, as current oslo libraries
don't export this namespace any longer, so other
tests will fail and catch this problem.
Related: I49c74ade374582f53a3678a1bc7df194c24e892e
Change-Id: I2af1679f1e2e82ba18d01e092ff8d01fa1537ca1
This reverts commit 9400cee06f.
Calling assert_called_once seems to fail as expected
using mock 1.3.0.
Change-Id: Ie382989e03b31564a3bfb4e8d010a8a93467f71b
We've had a few patches now over the last few releases to fix invalid
mock assertions for assert_called_once. Rather than keep letting these
creep back in to the code, this adds a hacking check to watch for this
specific call and block it.
Change-Id: I3cf4df523ddba49f57dd1d4d23e95e6d62d03c9e
This patch adds a hacking check to make sure that assertEquals isn't
comparing the result value to True or None. When developers
use assertEquals(None, return_value) the check will catch it when
pep8 runs and suggest using assertIsNone(return_value) instead.
Similar situations will occur when trying to use assertTrue(True,
return_value).
This patch also makes the necessary changes that get caught by the
new hacking check.
Change-Id: I56cc8121784eee617c09fb4e92b4ebb5877a0553
Depending on how opts are registered (either with register_opt() or
register_opts()), the name needs to be singular or plural to match
the method. This patch adds a hacking check to make sure the names
of the opts and opt lists (or tuples) are correct given how they are
being registered. The check also verifies that a single option is
sent when register_opt() is used and a list is used when using
register_opts().
Includes fixes to files that don't meet the naming convention and a
addition to the generate_cinder_opts.py file in order to skip
checks.py in the generation of the opts.py file.
Closes-Bug: 1495752
Change-Id: Ia795915c6e3d46272acc30407961d5d876f783ce
Added hacking check for log lines that accidently pass a tuple as the
second argument when providing format arguments.
Change-Id: Ifba23948f9fb49e9f6a1ee7e743aba0cd864c827
Closes-Bug: 1415241
There are some barbarism located in the file cinder/HACKING.rst.
line11 and line21 : there is no full point at the end of a sentence.
line14 and line23 : after a sentence, there are two space between
the full point and the first letter of the second sentence.
I think the correct format of the above question as blow.
- [N319] Validate that debug level logs are not translated.
- [C302] six.text_type should be used instead of unicode.
- [N325] str() and unicode() cannot be used on
an exception. Remove or use six.text_type().
- [C304] Enforce no use of LOG.audit
messages. LOG.info should be used instead.
Change-Id: I2896bacc5379786ed115d47e520bf5bc2af0028a
Closes-Bug: #1487665
It has been discussed that there really isn't much point in having
unit tests making any kind of logger calls. Some usage has already
been cleaned up. This patch removes the remaining instances of log
calls under the cinder/tests directory.
Also cleaned up a lot of cases where the source files would import
oslo_logging and declare a LOG variable which was never actually
used.
Leaving logging in the cinder/tests/unit/integrated tree for now
until a plan is decided as to what to do with all of these type of
tests.
Added hacking check to prevent new instances from slipping by code
reviews.
Change-Id: If177394486d5c92fa5466cd3825b15d30cf5fb18
With mock 1.1.0+, mock will fail when nonexistent
methods are called, so this check is no longer
necessary.
Related-Bug: #1473454
Change-Id: I9cb8b4a5eab78e51728aa8f83668f5979c0b9be1
PEP-0274 introduced dict comprehensions to replace dict constructor
with a sqeuence of key-pairs[1], these are benefits:
First, it makes the code look neater.
Second, it gains a micro-optimization.
Cinder dropped python 2.6 support in Kilo, we can leverage this now.
Note: This commit doesn't handle dict constructor with kwargs.
This commit also adds a hacking rule.
[1]http://legacy.python.org/dev/peps/pep-0274/
Change-Id: Ie65796438160b1aa1f47c8519fb9943e81bff3e9
LOG.warn is deprecated and LOG.warning should be used.
This patch fixes up instances of LOG.warn usage and adds a
hacking check to make sure it doesn't creep back in.
See Logger.warning note here for background:
https://docs.python.org/3/library/logging.html
Also cleaned up some remaining instances where logging was
preformatting strings rather than passing in formatting
arguments to the logger to handle.
Change-Id: Id2e6cba489d8509601820b5aed83652f71be2bdc
One of the comments we are frequently having to make
on reviews is with regards to the use of str() or
unicode() on exceptions. This hacking check pulled
from Nova will catch this problem earlier and avoid
conflicting comments being made in reviews.
Change-Id: I09eaf7b066f3e29733e6cbe16b2997edb6bc5e23
This patch remove some code that tried to parse time in different way by
leveraging timeutils instead. It also drops strtime() usage as it's
going to be deprecated in oslo_utils (see
I8b5119e64369ccac3423dccc04421f99912df733).
Change-Id: Icb2906ccd4b381f80064e0f1348fc64e389031dd
We have a couple of hacking checks that are specific to
Cinder that were written a while back. Unfortunately, when
they were written the numbering scheme for hacking checks was
not understood. We used N3xx when we should have used C3xx.
This patch corrects that mistake.
Change-Id: Ia17797005d444ab53a45b47b80b97799114001ee
Closes-bug: 1440833
We are frequently having to -1 patches because people
forget print() statements that were used for debug in
their development. Can save everyone time and trouble by
adding this simple hacking check.
The check excluded the cinder/cmd directory as the files in there
legitimately need to use the print() command. Also wsgi.py and
the test_hds_nas_backend.py files make use of print, so I have
excluded those from checking as well.
Change-Id: I2066eeb2bdc6c9af294d0b9befb7e0b32abd1378
Part of multi-patch set for easier chunks.
There have been quite a few instances found where the
i18n guidelines are not being followed. I believe this
has helped lead to some of the confusion around how to
correctly do this. Other developers see this code and
assume it is an example of the correct usage.
This patch attempts to clean up most of those violations
in the existing codebase to hopefully help avoid some of
that confusion in reviews.
Some issues address:
* Correct log translation markers for different log levels
* Passing format values as arguments to call, not preformatting
* Not forcing translation via six.text_type and others
Guidelines can be found here:
http://docs.openstack.org/developer/oslo.i18n/guidelines.html
Hacking checks will not be able to identify all violations of
the guidelines, but it could be useful for catching obvious ones
such as LOG.info("No markers!").
Change-Id: I38f52c6408b47ccb59ec2064b360f7d4427d6830
Partial-bug: 1433216
The contextlib.nested call has been deprecated
in Python 2.7. This causes DeprecationWarning
messages in the unit tests.
There are also known issues with contextlib.nested
that were addressed by the native support for
multiple "with" variables. For instance, if the
first object is created but the second one throws
an exception, the first object's __exit__ is never
called.
Since Cinder no longer supports 2.6 we can remove
the use of these contextlib.nested calls.
Added hacking check to catch if any new instances
are added to the codebase.
Note: line continuation markers (e.g. '\') had to
be used or syntax errors were thrown. While using
parentheses is the preferred way for multiple line
statements it is not a requirement.
Partial-Bug: 1428424
Change-Id: I7bb7d201d31ff239be3402fb64e5f202ede019b0
We want to make sure that we don't have usage of the old
oslo.concurrency naming slipping in with new changes where
we should be using the oslo_concurrency namespace. This change
adds a hacking check to avoid use of the deprecated namespace.
As we convert more oslo libraries to the new namespace the check
will be updated to enforce use of the new namespace.
This hacking check is based upon the same N333 hacking check
in Cinder.
Change-Id: Ibec6d09e9d313c9e723f7542cedb9da5772d3de2
Mock has a method called assert_called_once_with to check that a mock
was called and the arguments it took were as expected. Mock does not
have a method called assert_called_once and calling it just creates a
mock bound to that name. This means that not only is nothing tested
when assert_called_once is used, the tests also don't warn about this.
This commit attempts to address this in two ways:
- all occurrences of assert_called_once are replaced with a real
assertion.
- the hacking check that nova uses to guard against this has been
copied to cinder's local hacking checks.
Fixing the assert_called_once issues also highlighted other mistakes
in certain tests which were addressed to make the tests pass.
Due to the nature of mock, this issue is also possible if a method is
misspelt or just mistakenly used and so the hacking check is only
addressing one very specific case. That said, it does appear to be a
common mistake and so is worth singling out.
Change-Id: Iedcc3f48d91f7ebd8878ccc3bca3d023503774bd
Closes-Bug: #1394544
We haven't been very good at enforcing putting new hacking checks
into HACKING.rst. This change brings the file up to date.
Change-Id: I95ef533d031d333da5615ead997b54c6506c2c2a
Commit 4dc37abc removes the few instances of LOG.audit that
were in Cinder. Given that the plan is to remove LOG.audit messages
from OpenStack, I am adding this hacking check to ensure that such
messages do not sneak their way back into Cinder.
Unit tests are included with this change.
Change-Id: Icc416a68f958f60260f1c55af0d8605c95913bf1
Commit 3e2b1117 added a check to make sure that the _
function was being explicitly imported so that translation would
work properly.
I have discovered that those regexes/code would not work in some cases.
Particularly if the import line imported multiple things from
gettextutils or i18n. Also the check being used to find lines using
the _ function was not right.
This commit fixes the issues and adds appropriate tests. It also
adds the hacking check to HACKING.rst which should have been done the
first time around.
Change-Id: I7227bb0051836e537bff2f0f97662c06452d5af6
According to the OpenStack translation policy available at
https://wiki.openstack.org/wiki/LoggingStandards debug messages
should not be translated. Like mentioned in several changes in
Nova by garyk this is to help prioritize log translation.
This patch adds a new hacking check - N319 - that ensures all
debug log messages don't have translations.
Change-Id: Id9c2715f25c8f2ea52235aba4bd1583655391584
Implements: blueprint debug-translation-removal
Closes-Bug: #1318713
The current link in the HACKING file is broken. This references the
correct location for contributors to view.
Change-Id: I614f78fdea32025c2c5cf9599c698dde9c81ab21
Reference the OpenStack hacking guide in HACKING.rst and remove
duplicate entries. Add placeholder section for cinder specific rules.
cinder specific rules can be created using hacking's local check
support.
Change-Id: Ia74da70363e3fe602405a440c1d2ec75052e9193
Raising NEW exception is bad practice, because we lose TraceBack.
So all places like:
except SomeException as e:
raise e
should be replaced by
except SomeException:
raise
If we are doing some other actions before reraising we should
store information about exception then do all actions and then
reraise it. This is caused by eventlet bug. It lost information
about exception if it switch threads.
fixes bug 1191730
Change-Id: Ic2be96e9f03d2ca46d060caf6f6f7f713a1d6b82
Eric Harney