Re: [PATCH v2 0/4] Fix python style issues
by Vojtech Juranek
> The whole patch looks good!
+1
> On Sun, Jun 16, 2019 at 7:12 PM Nir Soffer <nirsof(a)gmail.com> wrote:
> > Fix python style issues and add flake8 style check to the build to keep
> > the code in good shape.
> >
> > Changes since v1:
> > - Add flake8 to travis build
> > - Replace star imports with explicit imports to allow flake8 detection
> >
> > of undefined names (Pavel)
> >
> > - Convert example.py to python3 - breaks flake8 on python 3 builds.
> >
> > v1 was here:
> >
> > https://lists.fedorahosted.org/archives/list/sanlock-devel@lists.fedorahos
> > ted.org/thread/ZH4XT326TS25PCZH2ESGVME5XPSUZPUR/>
> > Nir Soffer (4):
> > python: Add flak8 env
> > python: Avoid star imports
> > python: Fix style issues
> > python: Convert example.py to python 3
> >
> > .travis.yml | 4 ++++
> > README.dev | 4 ++++
> > python/example.py | 56 +++++++++++++++++++++++++++-----------------
> > tests/conftest.py | 8 +++----
> > tests/daemon_test.py | 15 ++++++++----
> > tests/direct_test.py | 2 +-
> > tests/python_test.py | 48 +++++++++++++++++++++++++------------
> > tests/util.py | 2 +-
> > tox.ini | 9 ++++++-
> > 9 files changed, 101 insertions(+), 47 deletions(-)
> >
> > --
> > 2.17.2
4 years, 9 months
[sanlock] 04/04: python: Convert example.py to python 3
by pagure@pagure.io
This is an automated email from the git hooks/post-receive script.
nsoffer pushed a commit to branch master
in repository sanlock.
commit 8bf78b8468d21d9adca78235e6e29a2235589e08
Author: Nir Soffer <nsoffer(a)redhat.com>
AuthorDate: Sun Jun 16 17:47:52 2019 +0300
python: Convert example.py to python 3
Use print function instead of print statement to make the example
compatible with both python 2 and 3. Tested only by flake8, may not
actually work with python 3.
Signed-off-by: Nir Soffer <nsoffer(a)redhat.com>
---
python/example.py | 30 ++++++++++++++++--------------
1 file changed, 16 insertions(+), 14 deletions(-)
diff --git a/python/example.py b/python/example.py
index 1be7e66..35d39f5 100644
--- a/python/example.py
+++ b/python/example.py
@@ -1,3 +1,5 @@
+from __future__ import print_function
+
import pwd
import grp
import os
@@ -12,13 +14,13 @@ RESOURCE_NAME = "resource1"
def sigTermHandler():
- print "SIGTERM signal received"
+ print("SIGTERM signal received")
def main():
signal.signal(signal.SIGTERM, sigTermHandler)
- print "Creating the sanlock disk"
+ print("Creating the sanlock disk")
fd, disk = tempfile.mkstemp()
os.close(fd)
@@ -28,33 +30,33 @@ def main():
SNLK_DISKS = [(disk, offset)]
- print "Registering to sanlock"
+ print("Registering to sanlock")
fd = sanlock.register()
- print "Initializing '%s'" % (LOCKSPACE_NAME,)
+ print("Initializing '%s'" % (LOCKSPACE_NAME,))
sanlock.write_lockspace(LOCKSPACE_NAME, disk, align=1048576, sector=512)
- print "Initializing '%s' on '%s'" % (RESOURCE_NAME, LOCKSPACE_NAME)
+ print("Initializing '%s' on '%s'" % (RESOURCE_NAME, LOCKSPACE_NAME))
sanlock.write_resource(
LOCKSPACE_NAME, RESOURCE_NAME, SNLK_DISKS, align=1048576, sector=512)
- print "Acquiring the id '%i' on '%s'" % (HOST_ID, LOCKSPACE_NAME)
+ print("Acquiring the id '%i' on '%s'" % (HOST_ID, LOCKSPACE_NAME))
sanlock.add_lockspace(LOCKSPACE_NAME, HOST_ID, disk)
try:
- print "Acquiring '%s' on '%s'" % (RESOURCE_NAME, LOCKSPACE_NAME)
+ print("Acquiring '%s' on '%s'" % (RESOURCE_NAME, LOCKSPACE_NAME))
sanlock.acquire(
LOCKSPACE_NAME, RESOURCE_NAME, SNLK_DISKS, slkfd=fd, version=0)
while True:
- print "Trying to get lockspace '%s' hosts" % LOCKSPACE_NAME
+ print("Trying to get lockspace '%s' hosts" % LOCKSPACE_NAME)
try:
hosts_list = sanlock.get_hosts(LOCKSPACE_NAME)
except sanlock.SanlockException as e:
if e.errno != os.errno.EAGAIN:
raise
else:
- print "Lockspace '%s' hosts: " % LOCKSPACE_NAME, hosts_list
+ print("Lockspace '%s' hosts: " % LOCKSPACE_NAME, hosts_list)
break
time.sleep(5)
@@ -64,17 +66,17 @@ def main():
SNLK_DISKS,
align=1048576,
sector=512)
- print "Resource '%s' owners: %s" % (RESOURCE_NAME, owners)
+ print("Resource '%s' owners: %s" % (RESOURCE_NAME, owners))
- print "Releasing '%s' on '%s'" % (RESOURCE_NAME, LOCKSPACE_NAME)
+ print("Releasing '%s' on '%s'" % (RESOURCE_NAME, LOCKSPACE_NAME))
sanlock.release(LOCKSPACE_NAME, RESOURCE_NAME, SNLK_DISKS, slkfd=fd)
except Exception as e:
- print "Exception: ", e
+ print("Exception: ", e)
finally:
- print "Releasing the id '%i' on '%s'" % (HOST_ID, LOCKSPACE_NAME)
+ print("Releasing the id '%i' on '%s'" % (HOST_ID, LOCKSPACE_NAME))
sanlock.rem_lockspace(LOCKSPACE_NAME, HOST_ID, disk)
- print "Removing the sanlock disk"
+ print("Removing the sanlock disk")
os.remove(disk)
--
To stop receiving notification emails like this one, please contact
the administrator of this repository.
4 years, 9 months
[sanlock] 03/04: python: Fix style issues
by pagure@pagure.io
This is an automated email from the git hooks/post-receive script.
nsoffer pushed a commit to branch master
in repository sanlock.
commit 1b230a8da337cb2ff386f02007b668a4e7fb467c
Author: Nir Soffer <nsoffer(a)redhat.com>
AuthorDate: Fri Jun 14 20:04:54 2019 +0300
python: Fix style issues
Fix style issues:
- 2 blank lines around functions and classes
- classes at the top of the module
- reformat to avoid long hard to read lines
- whitespace before ','
- whitespace after #
- unused imports
- unexpected indenation
- remove blank line at end of file
Signed-off-by: Nir Soffer <nsoffer(a)redhat.com>
---
python/example.py | 28 ++++++++++++++++++++--------
tests/conftest.py | 8 ++++----
tests/python_test.py | 50 ++++++++++++++++++++++++++++++++------------------
3 files changed, 56 insertions(+), 30 deletions(-)
diff --git a/python/example.py b/python/example.py
index 6ef9f7d..1be7e66 100644
--- a/python/example.py
+++ b/python/example.py
@@ -10,9 +10,11 @@ HOST_ID = 1
LOCKSPACE_NAME = "lockspace1"
RESOURCE_NAME = "resource1"
+
def sigTermHandler():
print "SIGTERM signal received"
+
def main():
signal.signal(signal.SIGTERM, sigTermHandler)
@@ -20,7 +22,8 @@ def main():
fd, disk = tempfile.mkstemp()
os.close(fd)
- os.chown(disk, pwd.getpwnam("sanlock").pw_uid, grp.getgrnam("sanlock").gr_gid)
+ os.chown(
+ disk, pwd.getpwnam("sanlock").pw_uid, grp.getgrnam("sanlock").gr_gid)
offset = sanlock.get_alignment(disk)
SNLK_DISKS = [(disk, offset)]
@@ -29,18 +32,20 @@ def main():
fd = sanlock.register()
print "Initializing '%s'" % (LOCKSPACE_NAME,)
- sanlock.write_lockspace(LOCKSPACE_NAME, disk, max_hosts=0, iotimeout=0, align=1048576, sector=512)
+ sanlock.write_lockspace(LOCKSPACE_NAME, disk, align=1048576, sector=512)
print "Initializing '%s' on '%s'" % (RESOURCE_NAME, LOCKSPACE_NAME)
- sanlock.write_resource(LOCKSPACE_NAME, RESOURCE_NAME, SNLK_DISKS, align=1048576, sector=512)
+ sanlock.write_resource(
+ LOCKSPACE_NAME, RESOURCE_NAME, SNLK_DISKS, align=1048576, sector=512)
print "Acquiring the id '%i' on '%s'" % (HOST_ID, LOCKSPACE_NAME)
sanlock.add_lockspace(LOCKSPACE_NAME, HOST_ID, disk)
try:
print "Acquiring '%s' on '%s'" % (RESOURCE_NAME, LOCKSPACE_NAME)
- sanlock.acquire(LOCKSPACE_NAME, RESOURCE_NAME, SNLK_DISKS, slkfd=fd,
- version=0)
+ sanlock.acquire(
+ LOCKSPACE_NAME, RESOURCE_NAME, SNLK_DISKS, slkfd=fd, version=0)
+
while True:
print "Trying to get lockspace '%s' hosts" % LOCKSPACE_NAME
try:
@@ -52,9 +57,15 @@ def main():
print "Lockspace '%s' hosts: " % LOCKSPACE_NAME, hosts_list
break
time.sleep(5)
- print "Resource '%s' owners: " % RESOURCE_NAME, \
- sanlock.read_resource_owners(
- LOCKSPACE_NAME, RESOURCE_NAME, SNLK_DISKS, align=1048576, sector=512)
+
+ owners = sanlock.read_resource_owners(
+ LOCKSPACE_NAME,
+ RESOURCE_NAME,
+ SNLK_DISKS,
+ align=1048576,
+ sector=512)
+ print "Resource '%s' owners: %s" % (RESOURCE_NAME, owners)
+
print "Releasing '%s' on '%s'" % (RESOURCE_NAME, LOCKSPACE_NAME)
sanlock.release(LOCKSPACE_NAME, RESOURCE_NAME, SNLK_DISKS, slkfd=fd)
except Exception as e:
@@ -66,5 +77,6 @@ def main():
print "Removing the sanlock disk"
os.remove(disk)
+
if __name__ == '__main__':
main()
diff --git a/tests/conftest.py b/tests/conftest.py
index 70e83d4..868ae46 100644
--- a/tests/conftest.py
+++ b/tests/conftest.py
@@ -4,7 +4,6 @@ Fixtures for sanlock testing.
from __future__ import absolute_import
import os
-import stat
import pytest
@@ -12,6 +11,10 @@ from . import storage
from . import util
+class SanlockIsRunning(Exception):
+ """ Raised if sanlock running when it should not """
+
+
@pytest.fixture
def sanlock_daemon():
"""
@@ -50,6 +53,3 @@ def user_4k_path(request):
def no_sanlock_daemon():
if util.sanlock_is_running():
raise SanlockIsRunning
-
-class SanlockIsRunning(Exception):
- pass
diff --git a/tests/python_test.py b/tests/python_test.py
index a032951..331e901 100644
--- a/tests/python_test.py
+++ b/tests/python_test.py
@@ -5,7 +5,6 @@ from __future__ import absolute_import
import errno
import io
-import struct
import time
from contextlib import contextmanager
@@ -36,7 +35,7 @@ SECTOR_SIZE_4K = 4 * KiB
FILE_NAMES = [
- #name, encoding
+ # name, encoding
("ascii", None),
(u"ascii", None),
(u"\u05d0", None),
@@ -49,21 +48,27 @@ LOCKSPACE_OR_RESOURCE_NAMES = [
# Python 2 also supports str.
pytest.param(
"\xd7\x90",
- marks=pytest.mark.skipif(six.PY3, reason="python 3 supports only bytes")),
+ marks=pytest.mark.skipif(
+ six.PY3,
+ reason="python 3 supports only bytes")),
# Python 2 also supports unicode with ascii content.
pytest.param(
u"ascii",
- marks=pytest.mark.skipif(six.PY3, reason="python 3 supports only bytes")),
+ marks=pytest.mark.skipif(
+ six.PY3,
+ reason="python 3 supports only bytes")),
]
-(a)pytest.mark.parametrize("filename, encoding" , FILE_NAMES)
-(a)pytest.mark.parametrize("size,offset", [
+
+(a)pytest.mark.parametrize("filename, encoding", FILE_NAMES)
+(a)pytest.mark.parametrize("size, offset", [
# Smallest offset.
(LOCKSPACE_SIZE, 0),
# Large offset.
(LARGE_FILE_SIZE, LARGE_FILE_SIZE - LOCKSPACE_SIZE),
])
-def test_write_lockspace(tmpdir, sanlock_daemon, filename, encoding, size, offset):
+def test_write_lockspace(
+ tmpdir, sanlock_daemon, filename, encoding, size, offset):
path = util.generate_path(tmpdir, filename, encoding)
util.create_file(path, size)
@@ -103,7 +108,11 @@ def test_write_lockspace_4k(user_4k_path, sanlock_daemon, align):
util.write_guard(user_4k_path, align)
sanlock.write_lockspace(
- b"ls_name", user_4k_path, iotimeout=1, align=align, sector=SECTOR_SIZE_4K)
+ b"ls_name",
+ user_4k_path,
+ iotimeout=1,
+ align=align,
+ sector=SECTOR_SIZE_4K)
ls = sanlock.read_lockspace(
user_4k_path, align=align, sector=SECTOR_SIZE_4K)
@@ -144,7 +153,8 @@ def test_read_lockspace_4k_invalid_sector_size(sanlock_daemon, user_4k_path):
# Large offset.
(LARGE_FILE_SIZE, LARGE_FILE_SIZE - MIN_RES_SIZE),
])
-def test_write_resource(tmpdir, sanlock_daemon, filename, encoding, size, offset):
+def test_write_resource(
+ tmpdir, sanlock_daemon, filename, encoding, size, offset):
path = util.generate_path(tmpdir, filename, encoding)
util.create_file(path, size)
disks = [(path, offset)]
@@ -177,9 +187,6 @@ def test_write_resource(tmpdir, sanlock_daemon, filename, encoding, size, offset
magic = util.read_magic(path, offset)
assert magic == constants.PAXOS_DISK_MAGIC
-
- # TODO: check more stuff here...
-
util.check_guard(path, size)
@@ -567,7 +574,8 @@ def test_add_lockspace_parse_args(no_sanlock_daemon, name, filename, encoding):
@pytest.mark.parametrize("name", LOCKSPACE_OR_RESOURCE_NAMES)
@pytest.mark.parametrize("filename,encoding", FILE_NAMES)
-def test_write_lockspace_parse_args(no_sanlock_daemon, name, filename, encoding):
+def test_write_lockspace_parse_args(
+ no_sanlock_daemon, name, filename, encoding):
path = util.generate_path("/tmp/", filename, encoding)
with raises_sanlock_errno():
sanlock.write_lockspace(name, path)
@@ -575,7 +583,8 @@ def test_write_lockspace_parse_args(no_sanlock_daemon, name, filename, encoding)
@pytest.mark.parametrize("name", LOCKSPACE_OR_RESOURCE_NAMES)
@pytest.mark.parametrize("filename,encoding", FILE_NAMES)
-def test_write_resource_parse_args(no_sanlock_daemon, name, filename, encoding):
+def test_write_resource_parse_args(
+ no_sanlock_daemon, name, filename, encoding):
path = util.generate_path("/tmp/", filename, encoding)
disks = [(path, 0)]
with raises_sanlock_errno():
@@ -587,7 +596,8 @@ def test_write_resource_parse_args(no_sanlock_daemon, name, filename, encoding):
@pytest.mark.parametrize("name", LOCKSPACE_OR_RESOURCE_NAMES)
@pytest.mark.parametrize("filename,encoding", FILE_NAMES)
-def test_release_resource_parse_args(no_sanlock_daemon, name, filename, encoding):
+def test_release_resource_parse_args(
+ no_sanlock_daemon, name, filename, encoding):
path = util.generate_path("/tmp/", filename, encoding)
disks = [(path, 0)]
with raises_sanlock_errno():
@@ -599,7 +609,8 @@ def test_release_resource_parse_args(no_sanlock_daemon, name, filename, encoding
@pytest.mark.parametrize("name", LOCKSPACE_OR_RESOURCE_NAMES)
@pytest.mark.parametrize("filename,encoding", FILE_NAMES)
-def test_read_resource_owners_parse_args(no_sanlock_daemon, name, filename, encoding):
+def test_read_resource_owners_parse_args(
+ no_sanlock_daemon, name, filename, encoding):
path = util.generate_path("/tmp/", filename, encoding)
disks = [(path, 0)]
with raises_sanlock_errno():
@@ -643,7 +654,8 @@ def test_set_event_parse_args(no_sanlock_daemon, name):
@pytest.mark.parametrize("name", LOCKSPACE_OR_RESOURCE_NAMES)
@pytest.mark.parametrize("filename,encoding", FILE_NAMES)
-def test_init_lockspace_parse_args(no_sanlock_daemon, name, filename, encoding):
+def test_init_lockspace_parse_args(
+ no_sanlock_daemon, name, filename, encoding):
path = util.generate_path("/tmp/", filename, encoding)
with raises_sanlock_errno(errno.ENODEV):
sanlock.init_lockspace(name, path)
@@ -659,21 +671,23 @@ def test_init_resource_parse_args(no_sanlock_daemon, name, filename, encoding):
with raises_sanlock_errno(errno.ENOENT):
sanlock.init_resource(name, b"res_name", disks)
+
@pytest.mark.parametrize("filename,encoding", FILE_NAMES)
def test_get_alignment_parse_args(no_sanlock_daemon, filename, encoding):
path = util.generate_path("/tmp/", filename, encoding)
with raises_sanlock_errno(errno.ENOENT):
sanlock.get_alignment(path)
+
@pytest.mark.parametrize("filename,encoding", FILE_NAMES)
def test_read_lockspace_parse_args(no_sanlock_daemon, filename, encoding):
path = util.generate_path("/tmp/", filename, encoding)
with raises_sanlock_errno():
sanlock.read_lockspace(path)
+
@pytest.mark.parametrize("filename,encoding", FILE_NAMES)
def test_read_resource_parse_args(no_sanlock_daemon, filename, encoding):
path = util.generate_path("/tmp/", filename, encoding)
with raises_sanlock_errno():
sanlock.read_resource(path)
-
--
To stop receiving notification emails like this one, please contact
the administrator of this repository.
4 years, 9 months
[sanlock] 02/04: python: Avoid star imports
by pagure@pagure.io
This is an automated email from the git hooks/post-receive script.
nsoffer pushed a commit to branch master
in repository sanlock.
commit 88940740848b189843d3541a71ec8cf5fc063020
Author: Nir Soffer <nsoffer(a)redhat.com>
AuthorDate: Sun Jun 16 16:58:07 2019 +0300
python: Avoid star imports
Star Imports (from module import *) are very convenient but flake8 is
not smart enough to tell which names are imported. Replace the star
imports with specific imports.
This changes fixes these flake8 errors:
5 F403 'from .constants import *' used; unable to detect undefined names
44 F405 'MiB' may be undefined, or defined from star imports: .constants, .units
Signed-off-by: Nir Soffer <nsoffer(a)redhat.com>
---
tests/daemon_test.py | 15 +++++++++++----
tests/direct_test.py | 2 +-
tests/python_test.py | 2 +-
tests/util.py | 2 +-
4 files changed, 14 insertions(+), 7 deletions(-)
diff --git a/tests/daemon_test.py b/tests/daemon_test.py
index 36648e3..413ce95 100644
--- a/tests/daemon_test.py
+++ b/tests/daemon_test.py
@@ -9,9 +9,17 @@ import struct
import pytest
-from . constants import *
+from . constants import (
+ DELTA_DISK_MAGIC,
+ PAXOS_DISK_MAGIC,
+ PAXOS_DISK_CLEAR,
+ RINDEX_DISK_MAGIC,
+ RINDEX_ENTRIES_SECTORS,
+ RINDEX_ENTRY_SIZE
+)
+
from . import util
-from . units import *
+from . units import MiB
def test_single_instance(sanlock_daemon):
@@ -126,8 +134,7 @@ def test_create(tmpdir, sanlock_daemon):
# New entry should be created at the first slot
# The first rindex sector is used by the rindex header.
f.seek(MiB + 512)
- util.check_rindex_entry(f.read(RINDEX_ENTRY_SIZE),
- b"res", 3 * MiB, 0)
+ util.check_rindex_entry(f.read(RINDEX_ENTRY_SIZE), b"res", 3 * MiB, 0)
# The rest of the entries should not be modified.
rest = 512 * RINDEX_ENTRIES_SECTORS - RINDEX_ENTRY_SIZE
diff --git a/tests/direct_test.py b/tests/direct_test.py
index a157bc4..339ef51 100644
--- a/tests/direct_test.py
+++ b/tests/direct_test.py
@@ -8,7 +8,7 @@ import struct
from . import constants
from . import util
-from . units import *
+from . units import MiB
def test_init_lockspace(tmpdir):
diff --git a/tests/python_test.py b/tests/python_test.py
index 8822521..a032951 100644
--- a/tests/python_test.py
+++ b/tests/python_test.py
@@ -17,7 +17,7 @@ import sanlock
from . import constants
from . import util
-from . units import *
+from . units import KiB, MiB, GiB, TiB
# Largest file size on ext4 is 16TiB, and on xfs 500 TiB. Use 1 TiB as it is
diff --git a/tests/util.py b/tests/util.py
index 243b1d3..6a648c4 100644
--- a/tests/util.py
+++ b/tests/util.py
@@ -11,7 +11,7 @@ import struct
import subprocess
import time
-from . units import *
+from . units import KiB
TESTDIR = os.path.dirname(__file__)
SANLOCK = os.path.join(TESTDIR, os.pardir, "src", "sanlock")
--
To stop receiving notification emails like this one, please contact
the administrator of this repository.
4 years, 9 months
[sanlock] 01/04: python: Add flak8 env
by pagure@pagure.io
This is an automated email from the git hooks/post-receive script.
nsoffer pushed a commit to branch master
in repository sanlock.
commit aa7d0bf5b0f196a9340163f0aff89f6dbdf349f0
Author: Nir Soffer <nsoffer(a)redhat.com>
AuthorDate: Fri Jun 14 19:53:17 2019 +0300
python: Add flak8 env
Flake8 provides basic linting and style checks for python code, helping
to keep the source in good shape.
Running tox will run flake8 automatically, showing errors in the source
and statistics for current issues. To run only flake8 tests use:
$ tox -e flake8
Running tox will fail now with these errors:
1 E203 whitespace before ','
1 E265 block comment should start with '# '
7 E302 expected 2 blank lines, found 1
1 E305 expected 2 blank lines after class or function definition, found 1
14 E501 line too long (82 > 79 characters)
1 F401 'stat' imported but unused
5 F403 'from .constants import *' used; unable to detect undefined names
44 F405 'MiB' may be undefined, or defined from star imports: .constants, .units
1 W391 blank line at end of file
Signed-off-by: Nir Soffer <nsoffer(a)redhat.com>
---
.travis.yml | 4 ++++
README.dev | 4 ++++
tox.ini | 9 ++++++++-
3 files changed, 16 insertions(+), 1 deletion(-)
diff --git a/.travis.yml b/.travis.yml
index 7b3fbe5..89fb52a 100644
--- a/.travis.yml
+++ b/.travis.yml
@@ -16,7 +16,11 @@ addons:
- libblkid-dev
- make
+install:
+ - pip install flake8
+
script:
- make BUILDARGS="--build-lib=."
- source tests/env.sh
- pytest
+ - flake8 --statistics tests python
diff --git a/README.dev b/README.dev
index cc2c613..8ade5e6 100644
--- a/README.dev
+++ b/README.dev
@@ -37,6 +37,10 @@ To run only tests matching the substring "foo":
$ tox -- -k foo
+To run basic lint and style check:
+
+ $ tox -e flake8
+
Testing 4K support
==================
diff --git a/tox.ini b/tox.ini
index 5a48b23..0e337ce 100644
--- a/tox.ini
+++ b/tox.ini
@@ -4,7 +4,7 @@
# and then run "tox" from this directory.
[tox]
-envlist = py27,py36
+envlist = py27,py36,flake8
skipsdist = True
skip_missing_interpreters = True
@@ -22,6 +22,10 @@ commands =
py36: make PY_VERSION=3.6 BUILDARGS="--build-lib={envsitepackagesdir}"
pytest {posargs}
+[testenv:flake8]
+deps = flake8
+commands = flake8 --statistics tests python
+
[pytest]
# Notes:
# --basetemp: we must use /var/tmp as sanlock uses direct I/O.
@@ -29,3 +33,6 @@ commands =
# -rxs: show extra test summary: (s)skipped, (x)failed
# --durations: show slowest test duration
addopts = -rxs -vv --basetemp=/var/tmp/sanlock --durations=10
+
+[flake8]
+show_source = True
--
To stop receiving notification emails like this one, please contact
the administrator of this repository.
4 years, 9 months
Re: [PATCH v2 3/8] python: Streamline initexception
by Nir Soffer
On Mon, Jun 17, 2019 at 12:47 PM Pavel Bar <pbar(a)redhat.com> wrote:
> Just something to consider: *maybe* adding "finally" label at the end and
> freeing all the local variables using "Py_CLEAR" in the reverse order of
> their initialization and always returning "excp" (initialized to NULL at
> the beginning) could be more readable. Just a thought.
>
The point of this patch is to remove the cleanup. Why keep the local
variable when we can get
rid of them quickly?
On Sun, Jun 16, 2019 at 11:50 PM Nir Soffer <nirsof(a)gmail.com> wrote:
>
>> When we create the sanlock expcetion we create a function, create a
>>
>
> expcetion --> exception
>
>
>> method from the function, create a dict from the method, and finally
>> create an exception from the dict. This simple flow does not require
>> goto for cleanup.
>>
>> Creating a dict is simpler and less error prone using Py_BuildValue().
>>
>> When we finish with unused pointers, it is safer to set them to NULL
>> using Py_CLEAR(), ensuring that we don't access freed memory.
>>
>> Signed-off-by: Nir Soffer <nsoffer(a)redhat.com>
>> ---
>> python/sanlock.c | 31 ++++++++++++-------------------
>> 1 file changed, 12 insertions(+), 19 deletions(-)
>>
>> diff --git a/python/sanlock.c b/python/sanlock.c
>> index 21a7a76..5c2fd1a 100644
>> --- a/python/sanlock.c
>> +++ b/python/sanlock.c
>> @@ -1741,33 +1741,26 @@ sanlock_exception = {
>> };
>>
>> static PyObject *
>> initexception(void)
>> {
>> - int rv;
>> - PyObject *dict, *func, *meth, *excp = NULL;
>> -
>> - if ((dict = PyDict_New()) == NULL)
>> - goto exit_fail;
>> -
>> - if ((func = PyCFunction_New(&sanlock_exception, NULL)) == NULL)
>> - goto exit_fail;
>> + PyObject *func = PyCFunction_New(&sanlock_exception, NULL);
>> + if (func == NULL)
>> + return NULL;
>>
>> - meth = PyObject_CallFunction((PyObject *) &PyProperty_Type, "O",
>> func);
>> - Py_DECREF(func);
>> + PyObject *meth = PyObject_CallFunction((PyObject *)
>> &PyProperty_Type, "O", func);
>> + Py_CLEAR(func);
>> if (meth == NULL)
>> - goto exit_fail;
>> -
>> - rv = PyDict_SetItemString(dict, sanlock_exception.ml_name, meth);
>> - Py_DECREF(meth);
>> - if (rv < 0)
>> - goto exit_fail;
>> + return NULL;
>>
>> - excp = PyErr_NewException("sanlock.SanlockException", NULL, dict);
>> + PyObject *dict = Py_BuildValue("{s:O}", sanlock_exception.ml_name,
>> meth);
>> + Py_CLEAR(meth);
>> + if (dict == NULL)
>> + return NULL;
>>
>> -exit_fail:
>> - Py_XDECREF(dict);
>> + PyObject *excp = PyErr_NewException("sanlock.SanlockException",
>> NULL, dict);
>> + Py_CLEAR(dict);
>>
>> return excp;
>> }
>>
>> static int
>> --
>> 2.17.2
>>
>>
4 years, 9 months
[sanlock] 02/02: tests: add tests for clearing resource
by pagure@pagure.io
This is an automated email from the git hooks/post-receive script.
nsoffer pushed a commit to branch master
in repository sanlock.
commit bd061bf4586d62223c8ebe61c487c1e83194de2b
Author: Vojtech Juranek <vjuranek(a)redhat.com>
AuthorDate: Mon Jun 17 12:39:07 2019 +0200
tests: add tests for clearing resource
Add tests for write_resource() with parameter 'clear' set to True, which
should result into clearing the resource (overwriting PAXOS_DISK_MAGIC
with PAXOS_DISK_CLEAR). Also add tests for corner cases like calling
clear on empty storage or with empty lockspace and resource name.
---
tests/constants.py | 4 ++++
tests/python_test.py | 47 +++++++++++++++++++++++++++++++++++++++++++++++
2 files changed, 51 insertions(+)
diff --git a/tests/constants.py b/tests/constants.py
index a7eb16b..21089b8 100644
--- a/tests/constants.py
+++ b/tests/constants.py
@@ -17,3 +17,7 @@ RINDEX_DISK_MAGIC = 0x01042018
RINDEX_ENTRY_SIZE = 64
RINDEX_ENTRIES_SECTORS = 2000
+
+# src/sanlock_rv.h
+
+SANLK_LEADER_MAGIC = -223
diff --git a/tests/python_test.py b/tests/python_test.py
index 58d94c9..8822521 100644
--- a/tests/python_test.py
+++ b/tests/python_test.py
@@ -227,6 +227,53 @@ def test_write_resource_4k_invalid_sector_size(sanlock_daemon, user_4k_path):
assert e.value.errno == errno.EINVAL
+def test_clear_resource(tmpdir, sanlock_daemon):
+ path = util.generate_path(tmpdir, "clear_test")
+ util.create_file(path, MiB)
+ disks = [(path, 0)]
+
+ sanlock.write_resource(b"ls_name", b"res_name", disks)
+ sanlock.write_resource(b"ls_name", b"res_name", disks, clear=True)
+
+ with pytest.raises(sanlock.SanlockException) as e:
+ sanlock.read_resource(path)
+ assert e.value.errno == constants.SANLK_LEADER_MAGIC
+
+ magic = util.read_magic(path)
+ assert magic == constants.PAXOS_DISK_CLEAR
+
+ util.check_guard(path, MiB)
+
+ # run clear on already cleared resource
+ sanlock.write_resource(b"ls_name", b"res_name", disks, clear=True)
+ magic = util.read_magic(path)
+ assert magic == constants.PAXOS_DISK_CLEAR
+
+
+def test_clear_empty_lockspace_resource(tmpdir, sanlock_daemon):
+ path = util.generate_path(tmpdir, "clear_test")
+ util.create_file(path, MiB)
+ disks = [(path, 0)]
+
+ sanlock.write_resource(b"ls_name", b"res_name", disks)
+
+ # Clear with empty lockspace and resource - should succeed
+ sanlock.write_resource(b"", b"", disks, clear=True)
+ magic = util.read_magic(path)
+ assert magic == constants.PAXOS_DISK_CLEAR
+
+
+def test_clear_empty_storage(tmpdir, sanlock_daemon):
+ path = util.generate_path(tmpdir, "clear_test")
+ util.create_file(path, MiB)
+ disks = [(path, 0)]
+
+ # Clear area without any resource written - should succeed
+ sanlock.write_resource(b"ls_name", b"inval_res_name", disks, clear=True)
+ magic = util.read_magic(path)
+ assert magic == constants.PAXOS_DISK_CLEAR
+
+
def test_read_resource_4k_invalid_sector_size(sanlock_daemon, user_4k_path):
disks = [(user_4k_path, 0)]
--
To stop receiving notification emails like this one, please contact
the administrator of this repository.
4 years, 9 months
[sanlock] 01/02: tests: replace reading of magic number by helper
function
by pagure@pagure.io
This is an automated email from the git hooks/post-receive script.
nsoffer pushed a commit to branch master
in repository sanlock.
commit 272221e6fcadda71b95487931286666da810ceda
Author: Vojtech Juranek <vjuranek(a)redhat.com>
AuthorDate: Mon Jun 17 12:39:06 2019 +0200
tests: replace reading of magic number by helper function
Introduce helper function for reading sanlock magic numbers. This
helper function seeks to specified offset in given file and reads
unsigned int from this position.
---
tests/python_test.py | 25 +++++++++----------------
tests/util.py | 11 +++++++++++
2 files changed, 20 insertions(+), 16 deletions(-)
diff --git a/tests/python_test.py b/tests/python_test.py
index 3242f57..58d94c9 100644
--- a/tests/python_test.py
+++ b/tests/python_test.py
@@ -86,12 +86,9 @@ def test_write_lockspace(tmpdir, sanlock_daemon, filename, encoding, size, offse
b"ls_name", 1, path, offset=offset, wait=False)
assert acquired is False
- with io.open(path, "rb") as f:
- f.seek(offset)
- magic, = struct.unpack("< I", f.read(4))
- assert magic == constants.DELTA_DISK_MAGIC
-
- # TODO: check more stuff here...
+ magic = util.read_magic(path, offset)
+ assert magic == constants.DELTA_DISK_MAGIC
+ # TODO: check more stuff here...
util.check_guard(path, size)
@@ -117,9 +114,8 @@ def test_write_lockspace_4k(user_4k_path, sanlock_daemon, align):
assert acquired is False
# Verify that lockspace was written.
- with io.open(user_4k_path, "rb") as f:
- magic, = struct.unpack("< I", f.read(4))
- assert magic == constants.DELTA_DISK_MAGIC
+ magic = util.read_magic(user_4k_path)
+ assert magic == constants.DELTA_DISK_MAGIC
# Check that sanlock did not write beyond the lockspace area.
util.check_guard(user_4k_path, align)
@@ -179,10 +175,8 @@ def test_write_resource(tmpdir, sanlock_daemon, filename, encoding, size, offset
owners = sanlock.read_resource_owners(b"ls_name", b"res_name", disks)
assert owners == []
- with io.open(path, "rb") as f:
- f.seek(offset)
- magic, = struct.unpack("< I", f.read(4))
- assert magic == constants.PAXOS_DISK_MAGIC
+ magic = util.read_magic(path, offset)
+ assert magic == constants.PAXOS_DISK_MAGIC
# TODO: check more stuff here...
@@ -216,9 +210,8 @@ def test_write_resource_4k(sanlock_daemon, user_4k_path, align):
assert owners == []
# Verify that resource was written.
- with io.open(user_4k_path, "rb") as f:
- magic, = struct.unpack("< I", f.read(4))
- assert magic == constants.PAXOS_DISK_MAGIC
+ magic = util.read_magic(user_4k_path)
+ assert magic == constants.PAXOS_DISK_MAGIC
# Check that sanlock did not write beyond the lockspace area.
util.check_guard(user_4k_path, align)
diff --git a/tests/util.py b/tests/util.py
index 08db236..243b1d3 100644
--- a/tests/util.py
+++ b/tests/util.py
@@ -177,3 +177,14 @@ def sanlock_is_running():
if e.errno != errno.ENOENT:
raise
return False
+
+
+def read_magic(path, offset=0):
+ """
+ Read and return unsigned int from specified file at given offset.
+ Typical usage is to read sanlock magic number.
+ """
+ with io.open(path, "rb") as f:
+ f.seek(offset)
+ uint, = struct.unpack("< I", f.read(4))
+ return uint
--
To stop receiving notification emails like this one, please contact
the administrator of this repository.
4 years, 9 months
Re: [PATCH v2 4/8] python: Extract create_resource() helper
by Vojtech Juranek
On pondělí 17. června 2019 12:04:35 CEST Pavel Bar wrote:
> On Sun, Jun 16, 2019 at 11:50 PM Nir Soffer <nirsof(a)gmail.com> wrote:
> > Replace duplicate code for allocating and initializing sanlk_resource
> > struct with a helper.
> >
> > Signed-off-by: Nir Soffer <nsoffer(a)redhat.com>
> > ---
> >
> > python/sanlock.c | 46 +++++++++++++++++++++++-----------------------
> > 1 file changed, 23 insertions(+), 23 deletions(-)
> >
> > diff --git a/python/sanlock.c b/python/sanlock.c
> > index 5c2fd1a..6824e01 100644
> > --- a/python/sanlock.c
> > +++ b/python/sanlock.c
> > @@ -170,29 +170,38 @@ parse_single_disk(PyObject* disk, struct sanlk_disk*
> > res_disk)
> >
> > finally:
> > Py_XDECREF(path);
> > return rv;
> >
> > }
> >
> > +static struct sanlk_resource *
> > +create_resource(int num_disks)
> > +{
> > + size_t size = sizeof(struct sanlk_resource) +
> > + sizeof(struct sanlk_disk) * num_disks;
> > +
> > + struct sanlk_resource *res = calloc(1, size);
> > + if (res == NULL) {
> > + PyErr_NoMemory();
> > + return NULL;
> > + }
> > +
> > + res->num_disks = num_disks;
> > +
> > + return res;
> > +}
> > +
> >
> > static int
> > parse_disks(PyObject *obj, struct sanlk_resource **res_ret)
> > {
> >
> > - int num_disks, res_len;
> > + int num_disks;
> >
> > struct sanlk_resource *res;
>
> What about declaring the local variables when first used, thus immediately
> initializing them? I think C99 provides that.
> I.e.,
> int num_disks = PyList_Size(obj);
> struct sanlk_resource *res = create_resource(num_disks);
> Moreover, in this case they will be at the beginning of the block anyway.
agree, IMHO good idea to initialize variables once they are declared
> > num_disks = PyList_Size(obj);
> >
> > - res_len = sizeof(struct sanlk_resource) +
> > - (sizeof(struct sanlk_disk) * num_disks);
> > - res = malloc(res_len);
> > -
> > - if (res == NULL) {
> > - PyErr_NoMemory();
> > + res = create_resource(num_disks);
> > + if (res == NULL)
> >
> > return -1;
> >
> > - }
> > -
> > - memset(res, 0, res_len);
> > - res->num_disks = num_disks;
> >
> > for (int i = 0; i < num_disks; i++) {
> >
> > PyObject *disk = PyList_GetItem(obj,i);
> >
> > if (!parse_single_disk(disk, &(res->disks[i]))) {
> >
> > @@ -637,30 +646,21 @@ Align can be one of (1048576, 2097152, 4194304,
> > 8388608).\n\
> >
> > Sector can be one of (512, 4096).");
> >
> > static PyObject *
> > py_read_resource(PyObject *self __unused, PyObject *args, PyObject
> >
> > *keywds)
> >
> > {
> >
> > - int rv = -1, res_len, sector = SECTOR_SIZE_512;
> > + int rv = -1, sector = SECTOR_SIZE_512;
> >
> > long align = ALIGNMENT_1M;
> > PyObject *path = NULL;
> > struct sanlk_resource *res;
> > PyObject *res_info = NULL;
> >
> > static char *kwlist[] = {"path", "offset", "align", "sector", NULL};
> >
> > - /* allocate the needed memory for the resource and one disk */
> > - res_len = sizeof(struct sanlk_resource) + sizeof(struct sanlk_disk);
> > - res = malloc(res_len);
> > -
> > - if (res == NULL) {
> > - PyErr_NoMemory();
> > + res = create_resource(1 /* num_disks */);
>
> Not sure the comment is needed.
>
> > + if (res == NULL)
> >
> > return NULL;
> >
> > - }
> > -
> > - /* initialize resource and disk structures */
> > - memset(res, 0, res_len);
> > - res->num_disks = 1;
> >
> > /* parse python tuple */
> > if (!PyArg_ParseTupleAndKeywords(args, keywds, "O&|kli", kwlist,
> >
> > pypath_converter, &path, &(res->disks[0].offset), &align,
> >
> > §or)) {
> >
> > goto finally;
> >
> > --
> > 2.17.2
4 years, 9 months
Re: [PATCH v2 1/2] tests: replace reading of magic number by helper function
by Nir Soffer
On Sun, Jun 16, 2019 at 4:36 PM Pavel Bar <pbar(a)redhat.com> wrote:
>
>
> On Sat, Jun 15, 2019 at 1:02 AM Nir Soffer <nsoffer(a)redhat.com> wrote:
>
>> On Sat, Jun 15, 2019 at 12:55 AM Vojtech Juranek <vjuranek(a)redhat.com>
>> wrote:
>>
>>> Introduce helper function for reading sanlock magic numbers. This
>>> helper function seeks to specified offset in given file and reads
>>> unsigned int from this position.
>>> ---
>>> tests/python_test.py | 25 +++++++++----------------
>>> tests/util.py | 11 +++++++++++
>>> 2 files changed, 20 insertions(+), 16 deletions(-)
>>>
>>> diff --git a/tests/python_test.py b/tests/python_test.py
>>> index 3242f57..623a13c 100644
>>> --- a/tests/python_test.py
>>> +++ b/tests/python_test.py
>>> @@ -86,12 +86,9 @@ def test_write_lockspace(tmpdir, sanlock_daemon,
>>> filename, encoding, size, offse
>>> b"ls_name", 1, path, offset=offset, wait=False)
>>> assert acquired is False
>>>
>>> - with io.open(path, "rb") as f:
>>> - f.seek(offset)
>>> - magic, = struct.unpack("< I", f.read(4))
>>> - assert magic == constants.DELTA_DISK_MAGIC
>>> -
>>> - # TODO: check more stuff here...
>>> + magic = util.read_uint(path, offset)
>>> + assert magic == constants.DELTA_DISK_MAGIC
>>> + # TODO: check more stuff here...
>>>
>>> util.check_guard(path, size)
>>>
>>> @@ -117,9 +114,8 @@ def test_write_lockspace_4k(user_4k_path,
>>> sanlock_daemon, align):
>>> assert acquired is False
>>>
>>> # Verify that lockspace was written.
>>> - with io.open(user_4k_path, "rb") as f:
>>> - magic, = struct.unpack("< I", f.read(4))
>>> - assert magic == constants.DELTA_DISK_MAGIC
>>> + magic = util.read_uint(user_4k_path)
>>> + assert magic == constants.DELTA_DISK_MAGIC
>>>
>>> # Check that sanlock did not write beyond the lockspace area.
>>> util.check_guard(user_4k_path, align)
>>> @@ -179,10 +175,8 @@ def test_write_resource(tmpdir, sanlock_daemon,
>>> filename, encoding, size, offset
>>> owners = sanlock.read_resource_owners(b"ls_name", b"res_name",
>>> disks)
>>> assert owners == []
>>>
>>> - with io.open(path, "rb") as f:
>>> - f.seek(offset)
>>> - magic, = struct.unpack("< I", f.read(4))
>>> - assert magic == constants.PAXOS_DISK_MAGIC
>>> + magic = util.read_uint(path, offset)
>>> + assert magic == constants.PAXOS_DISK_MAGIC
>>>
>>> # TODO: check more stuff here...
>>>
>>> @@ -216,9 +210,8 @@ def test_write_resource_4k(sanlock_daemon,
>>> user_4k_path, align):
>>> assert owners == []
>>>
>>> # Verify that resource was written.
>>> - with io.open(user_4k_path, "rb") as f:
>>> - magic, = struct.unpack("< I", f.read(4))
>>> - assert magic == constants.PAXOS_DISK_MAGIC
>>> + magic = util.read_uint(user_4k_path)
>>> + assert magic == constants.PAXOS_DISK_MAGIC
>>>
>>> # Check that sanlock did not write beyond the lockspace area.
>>> util.check_guard(user_4k_path, align)
>>> diff --git a/tests/util.py b/tests/util.py
>>> index 08db236..32b34ec 100644
>>> --- a/tests/util.py
>>> +++ b/tests/util.py
>>> @@ -177,3 +177,14 @@ def sanlock_is_running():
>>> if e.errno != errno.ENOENT:
>>> raise
>>> return False
>>> +
>>> +
>>> +def read_uint(path, offset=0):
>>> + """
>>> + Read and return unsigned int from specified file at given offset.
>>> + Typical usage is to read sanlock magic number.
>>> + """
>>> + with io.open(path, "rb") as f:
>>> + f.seek(offset)
>>> + uint, = struct.unpack("< I", f.read(4))
>>> + return uint
>>> --
>>> 2.20.1
>>>
>>>
>> Looks good, but maybe call it read_magic(path, offset)?
>>
>
> Looks good.
> Regarding the Nir's comment, suggest "read_magic_int".
>
read_magic_init does not not add anything useful. We will never have
read_magic_float
so there is no reason to include the type in the name.
4 years, 9 months