[PATCH v2 0/4] Fix python style issues
by Nir Soffer
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.fedoraho...
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
Re: [PATCH 10/14] python: Remove unneeded check for null
by Nir Soffer
On Sun, Jun 16, 2019 at 6:40 PM Pavel Bar <pbar(a)redhat.com> wrote:
> I think there are more places like that in "sanlock.c".
>
git grep thinks differently:
$ git grep -B1 'free(' python
python/sanlock.c-exit_fail:
python/sanlock.c: free(res);
--
python/sanlock.c- Py_XDECREF(resource);
python/sanlock.c: free(res);
--
python/sanlock.c-finally:
python/sanlock.c: free(rs);
--
python/sanlock.c- Py_XDECREF(resource);
python/sanlock.c: free(rs);
--
python/sanlock.c- /* success */
python/sanlock.c: free(lss);
--
python/sanlock.c-exit_fail:
python/sanlock.c: if (lss) free(lss);
--
python/sanlock.c- Py_XDECREF(lockspace);
python/sanlock.c: free(hss);
--
python/sanlock.c- Py_XDECREF(resource);
python/sanlock.c: free(res);
--
python/sanlock.c- Py_XDECREF(resource);
python/sanlock.c: free(res);
--
python/sanlock.c- Py_XDECREF(resource);
python/sanlock.c: free(res);
--
python/sanlock.c- Py_XDECREF(resource);
python/sanlock.c: free(res);
python/sanlock.c: free(hss);
>
> On Fri, Jun 14, 2019 at 11:54 PM Nir Soffer <nirsof(a)gmail.com> wrote:
>
>> When calling free() we don't need to check if a point is NULL.
>>
>> Signed-off-by: Nir Soffer <nsoffer(a)redhat.com>
>> ---
>> python/sanlock.c | 2 +-
>> 1 file changed, 1 insertion(+), 1 deletion(-)
>>
>> diff --git a/python/sanlock.c b/python/sanlock.c
>> index ed9de0f..4c3e0a0 100644
>> --- a/python/sanlock.c
>> +++ b/python/sanlock.c
>> @@ -1025,11 +1025,11 @@ py_get_lockspaces(PyObject *self __unused,
>> PyObject *args, PyObject *keywds)
>> free(lss);
>> return ls_list;
>>
>> /* failure */
>> exit_fail:
>> - if (lss) free(lss);
>> + free(lss);
>> Py_XDECREF(ls_entry);
>> Py_XDECREF(ls_list);
>> return NULL;
>> }
>>
>> --
>> 2.17.2
>>
>>
4 years, 9 months
Re: [PATCH 2/2] python: Fix style issues
by Nir Soffer
On Sun, Jun 16, 2019 at 4:11 PM Pavel Bar <pbar(a)redhat.com> wrote:
>
>
> On Fri, Jun 14, 2019 at 8:15 PM Nir Soffer <nirsof(a)gmail.com> wrote:
>
>> 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
>> - 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 | 46 ++++++++++++++++++++++++++++++--------------
>> 3 files changed, 56 insertions(+), 26 deletions(-)
>>
>> diff --git a/python/example.py b/python/example.py
>> index 6ef9f7d..1be7e66 100644
>> --- a/python/example.py
>> +++ b/python/example.py
>> @@ -8,41 +8,46 @@ import sanlock
>>
>> HOST_ID = 1
>> LOCKSPACE_NAME = "lockspace1"
>> RESOURCE_NAME = "resource1"
>>
>> +
>> def sigTermHandler():
>> print "SIGTERM signal received"
>>
>> +
>> def main():
>> signal.signal(signal.SIGTERM, sigTermHandler)
>>
>> print "Creating the sanlock disk"
>> 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)
>>
>
> Not sure that the 2nd style is better, IMHO, it's even worse.
> The same long line with lot's of parameters with/without explicitly naming
> the receiving parameter's name.
> In my opinion if doing such a cleanup/improvement work, it should't be
> just "to mark a V" that some tool is not complaining anymore.
> In this case I believe it's better to split the line, so every parameter
> will appear in a single line as it really adds to readability. That's what
> you did below with "*owners = sanlock.read_resource_owners*..."
>
Using one argument per for all calls does not improve readability, so we
need a simple rule when
to more to one argument per line style.
My general indentation rules are:
1. use one line if it fits in one line
2. or move arguments to next line if it fits in 2 lines
3. or use one line per argument
This case matches rule 2.
offset = sanlock.get_alignment(disk)
>>
>> SNLK_DISKS = [(disk, offset)]
>>
>> print "Registering to sanlock"
>> 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)
>>
>
> Same note as above.
>
>
>>
>> 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:
>> hosts_list = sanlock.get_hosts(LOCKSPACE_NAME)
>> except sanlock.SanlockException as e:
>> @@ -50,13 +55,19 @@ def main():
>> raise
>> else:
>> 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)
>>
>
> Doesn't the "print " require "()" in Python 3? Don't we want to be Python
> 3 compatible (at least if we "touch" some code)?
>
Yes, the example code is not python 3 compatible yet. Converting it to
python 3 require
additional work, but it is not easy as this example is not easy to test.
+
>> 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
>> finally:
>> @@ -64,7 +75,8 @@ def main():
>> sanlock.rem_lockspace(LOCKSPACE_NAME, HOST_ID, disk)
>>
>> 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
>> @@ -2,18 +2,21 @@
>> Fixtures for sanlock testing.
>> """
>> from __future__ import absolute_import
>>
>> import os
>> -import stat
>>
>> import pytest
>>
>> from . import storage
>> from . import util
>>
>>
>> +class SanlockIsRunning(Exception):
>> + """ Raised if sanlock running when it should not """
>> +
>> +
>> @pytest.fixture
>> def sanlock_daemon():
>> """
>> Run sanlock daemon during a test.
>> """
>> @@ -48,8 +51,5 @@ def user_4k_path(request):
>>
>> @pytest.fixture
>> 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 3242f57..f37eab4 100644
>> --- a/tests/python_test.py
>> +++ b/tests/python_test.py
>> @@ -34,11 +34,11 @@ ALIGNMENT_2M = 2 * MiB
>> SECTOR_SIZE_512 = 512
>> SECTOR_SIZE_4K = 4 * KiB
>>
>>
>> FILE_NAMES = [
>> - #name, encoding
>> + # name, encoding
>> ("ascii", None),
>> (u"ascii", None),
>> (u"\u05d0", None),
>> (u"\u05d0", "utf-8"),
>> ]
>> @@ -47,25 +47,31 @@ LOCKSPACE_OR_RESOURCE_NAMES = [
>> # Bytes are supported with python 2 and 3.
>> pytest.param(b"\xd7\x90"),
>> # 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)
>>
>> # Test read and write with default alignment and sector size values.
>> sanlock.write_lockspace(b"ls_name", path, offset=offset, iotimeout=1)
>> @@ -104,11 +110,15 @@ def test_write_lockspace_4k(user_4k_path,
>> sanlock_daemon, align):
>> with io.open(user_4k_path, "rb+") as f:
>> f.write(align * b"x")
>> 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)
>>
>> assert ls == {"iotimeout": 1, "lockspace": b"ls_name"}
>> @@ -146,11 +156,12 @@ def
>> test_read_lockspace_4k_invalid_sector_size(sanlock_daemon, user_4k_path):
>> # Smallest offset.
>> (MIN_RES_SIZE, 0),
>> # 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)]
>>
>> # Test read and write with default alignment and sector size values.
>> @@ -525,19 +536,21 @@ def
>> test_add_lockspace_parse_args(no_sanlock_daemon, name, filename, encoding):
>> sanlock.add_lockspace(name, 1, path, 0, wait=False)
>>
>>
>> @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)
>>
>>
>> @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():
>> sanlock.write_resource(name, b"res_name", disks)
>>
>> @@ -545,11 +558,12 @@ def
>> test_write_resource_parse_args(no_sanlock_daemon, name, filename, encoding):
>> sanlock.write_resource(b"ls_name", name, disks)
>>
>>
>> @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():
>> sanlock.release(name, b"res_name", disks)
>>
>> @@ -557,11 +571,12 @@ def
>> test_release_resource_parse_args(no_sanlock_daemon, name, filename, encoding
>> sanlock.release(b"ls_name", name, disks)
>>
>>
>> @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():
>> sanlock.read_resource_owners(name, b"res_name", disks)
>>
>> @@ -601,11 +616,12 @@ def test_set_event_parse_args(no_sanlock_daemon,
>> name):
>> sanlock.set_event(name, 1, 1, 1)
>>
>>
>> @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)
>>
>>
>> @@ -617,23 +633,25 @@ def
>> test_init_resource_parse_args(no_sanlock_daemon, name, filename, encoding):
>> with raises_sanlock_errno(errno.ENOENT):
>> sanlock.init_resource(b"ls_name", name, disks)
>> 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)
>> -
>> --
>> 2.17.2
>>
>>
4 years, 9 months
Re: [PATCH 1/2] build: Add flak8 env
by Nir Soffer
On Sun, Jun 16, 2019 at 3:32 PM Pavel Bar <pbar(a)redhat.com> wrote:
>
>
> On Fri, Jun 14, 2019 at 8:15 PM Nir Soffer <nirsof(a)gmail.com> wrote:
>
>> 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
>>
>> We explicitly ignore errors about star import (from . units import *),
>> since these imports make the source much more readable.
>>
>> 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)
>
>
> What does it mean? What's the line limit? 79?
>
flake8 default max-line-length is 79.
>
>
>> 1 F401 'stat' imported but unused
>> 1 W391 blank line at end of file
>>
>> Signed-off-by: Nir Soffer <nsoffer(a)redhat.com>
>> ---
>> README.dev | 4 ++++
>> tox.ini | 12 +++++++++++-
>> 2 files changed, 15 insertions(+), 1 deletion(-)
>>
>> diff --git a/README.dev b/README.dev
>> index cc2c613..8ade5e6 100644
>> --- a/README.dev
>> +++ b/README.dev
>> @@ -35,10 +35,14 @@ To run only test from some modules:
>>
>> To run only tests matching the substring "foo":
>>
>> $ tox -- -k foo
>>
>> +To run basic lint and style check:
>> +
>> + $ tox -e flake8
>> +
>>
>> Testing 4K support
>> ==================
>>
>> To enable the 4k tests, you need to setup 4k stroage for the tests:
>>
>
> Typo. "storage"
>
Not related to this patch.
>
>
>> diff --git a/tox.ini b/tox.ini
>> index 5a48b23..ca63f55 100644
>> --- a/tox.ini
>> +++ b/tox.ini
>> @@ -2,11 +2,11 @@
>> # in multiple virtualenvs. This configuration file will run the
>> # test suite on all supported python versions. To use it, "pip install
>> tox"
>> # and then run "tox" from this directory.
>>
>> [tox]
>> -envlist = py27,py36
>> +envlist = py27,py36,flake8
>> skipsdist = True
>> skip_missing_interpreters = True
>>
>> [testenv]
>> passenv = *
>> @@ -20,12 +20,22 @@ deps =
>> commands =
>> py27: make PY_VERSION=2.7
>> BUILDARGS="--build-lib={envsitepackagesdir}"
>> 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.
>> # -vv: increasing verbosify twice shows more detailed failures
>> tracebacks.
>> # -rxs: show extra test summary: (s)skipped, (x)failed
>> # --durations: show slowest test duration
>> addopts = -rxs -vv --basetemp=/var/tmp/sanlock --durations=10
>> +
>> +[flake8]
>> +# We like to use start imports units and constants.
>>
>
> Typo. "star"
>
Thanks, will fix before pushing.
>
>> +# F403 'from .units import *' used; unable to detect undefined names
>> +# F405 'KiB' may be undefined, or defined from star imports: .units
>>
>
> Just want to understand:
> 1) F403 - That means that all "import *" except "from .units" will fail
> the build? If so, this is great, I don't like "import *" :)
> 2) F405 - I understand that we explicitly ignore "KiB" error. Why? When
> the error happens? What about other constants?
>
The error was copied from flake8 --statistics output, so it is confusing.
F403 ignores errors about star imports, which is fine. F405 ignores errors
about undefined
variables, which is less useful.
I will remove these ignores in v3, since we really like to have warnings
about undefined variables.
>
>
>> +ignore = F403, F405
>> --
>> 2.17.2
>>
>>
4 years, 9 months
Re: [PATCH] spec: Convert development spec to python 3
by Nir Soffer
On Sun, Jun 16, 2019 at 2:21 PM Pavel Bar <pbar(a)redhat.com> wrote:
> Looks good. Just 2 points to consider, see below:
>
>
> On Wed, Jun 12, 2019 at 10:04 PM Nir Soffer <nirsof(a)gmail.com> wrote:
>
>> The development spec creates now only python 3 packages. This will be
>> the base for Fedora 30 and CentOS 8.1 spec.
>>
>> Also make rpmlint happy about mixing tabs and spaces by using only
>> spaces.
>>
>> Signed-off-by: Nir Soffer <nsoffer(a)redhat.com>
>> ---
>> sanlock.spec.in | 42 +++++++++++++++++-------------------------
>> 1 file changed, 17 insertions(+), 25 deletions(-)
>>
>> diff --git a/sanlock.spec.in b/sanlock.spec.in
>> index 9ad2985..087076c 100644
>> --- a/sanlock.spec.in
>> +++ b/sanlock.spec.in
>> @@ -16,30 +16,23 @@ URL: https://pagure.io/sanlock/
>> BuildRequires: gcc
>> BuildRequires: libaio-devel
>> BuildRequires: libblkid-devel
>> BuildRequires: libuuid-devel
>> BuildRequires: make
>> -BuildRequires: python2
>> -BuildRequires: python2-devel
>> +BuildRequires: python3
>> +BuildRequires: python3-devel
>> BuildRequires: systemd-units
>> Requires: %{name}-lib = %{version}-%{release}
>> Requires(pre): /usr/sbin/groupadd
>> Requires(pre): /usr/sbin/useradd
>> Requires(post): systemd-units
>> Requires(post): systemd-sysv
>> Requires(preun): systemd-units
>> Requires(postun): systemd-units
>> Source0:
>> https://releases.pagure.org/sanlock/%{name}-%{version}.tar.gz
>>
>> -# On Fedora we switched sanlock-python to python2-sanlock, as part of
>> python 3
>> -# porting. This change was not done yet for RHEL.
>> -# TODO: remove when RHEL switches to pythonN-sanlock.
>> -%if 0%{?fedora}
>> -%global python_package python2-%{name}
>> -%else # centos/rhel
>> -%global python_package %{name}-python
>> -%endif
>> +%global python_package python3-%{name}
>>
>> %description
>> The sanlock daemon manages leases for applications on hosts using shared
>> storage.
>>
>> %prep
>> @@ -51,11 +44,11 @@ The sanlock daemon manages leases for applications on
>> hosts using shared storage
>> %endif
>> # upstream does not require configure
>> # upstream does not support _smp_mflags
>> CFLAGS=$RPM_OPT_FLAGS make -C wdmd
>> CFLAGS=$RPM_OPT_FLAGS make -C src
>> -CFLAGS=$RPM_OPT_FLAGS make -C python
>> +CFLAGS=$RPM_OPT_FLAGS make -C python PY_VERSION=3.6
>> CFLAGS=$RPM_OPT_FLAGS make -C reset
>>
>> %install
>> rm -rf $RPM_BUILD_ROOT
>> make -C src \
>> @@ -65,11 +58,11 @@ make -C wdmd \
>> install LIBDIR=%{_libdir} \
>> DESTDIR=$RPM_BUILD_ROOT
>> make -C python \
>> install LIBDIR=%{_libdir} \
>> DESTDIR=$RPM_BUILD_ROOT \
>> - PY_VERSION=2.7 # TODO: fix PY_VERSION to 3.6 when 2020 will
>> arrive
>> + PY_VERSION=3.6
>>
>
> 2nd usage of the hardcoded "3.6" Python version. Consider declaring a
> constant to avoid the situation that someone in the future might change
> only one of the appearances.
>
Good idea, I wasted few minutes while preparing this patch when one call
was using 3.6, and the other 2.7.
>
>
>> make -C reset \
>> install LIBDIR=%{_libdir} \
>> DESTDIR=$RPM_BUILD_ROOT
>>
>>
>> @@ -77,28 +70,28 @@ install -D -m 0644 init.d/sanlock.service.native
>> $RPM_BUILD_ROOT/%{_unitdir}/san
>> install -D -m 0755 init.d/wdmd
>> $RPM_BUILD_ROOT/usr/lib/systemd/systemd-wdmd
>> install -D -m 0644 init.d/wdmd.service.native
>> $RPM_BUILD_ROOT/%{_unitdir}/wdmd.service
>> install -D -m 0644 init.d/sanlk-resetd.service
>> $RPM_BUILD_ROOT/%{_unitdir}/sanlk-resetd.service
>>
>> install -D -m 0644 src/logrotate.sanlock \
>> - $RPM_BUILD_ROOT/etc/logrotate.d/sanlock
>> + $RPM_BUILD_ROOT/etc/logrotate.d/sanlock
>>
>> install -D -m 0644 src/sanlock.conf \
>> - $RPM_BUILD_ROOT/etc/sanlock/sanlock.conf
>> + $RPM_BUILD_ROOT/etc/sanlock/sanlock.conf
>>
>> install -D -m 0644 init.d/wdmd.sysconfig \
>> - $RPM_BUILD_ROOT/etc/sysconfig/wdmd
>> + $RPM_BUILD_ROOT/etc/sysconfig/wdmd
>>
>> install -Dd -m 0755 $RPM_BUILD_ROOT/etc/wdmd.d
>> install -Dd -m 0775 $RPM_BUILD_ROOT/%{_rundir}/sanlock
>> install -Dd -m 0775 $RPM_BUILD_ROOT/%{_rundir}/sanlk-resetd
>>
>> %pre
>> getent group sanlock > /dev/null || /usr/sbin/groupadd \
>> - -g 179 sanlock
>> + -g 179 sanlock
>> getent passwd sanlock > /dev/null || /usr/sbin/useradd \
>> - -u 179 -c "sanlock" -s /sbin/nologin -r \
>> - -g 179 -d /run/sanlock sanlock
>> + -u 179 -c "sanlock" -s /sbin/nologin -r \
>> + -g 179 -d /run/sanlock sanlock
>> /usr/sbin/usermod -a -G disk sanlock
>>
>> %post
>> %systemd_post wdmd.service sanlock.service
>>
>> @@ -144,25 +137,21 @@ access to the shared disks.
>>
>> %package -n %{python_package}
>> Summary: Python bindings for the sanlock library
>> Requires: %{name}-lib = %{version}-%{release}
>> %if 0%{?fedora}
>> -%{?python_provide:%python_provide python2-%{name}}
>> -# Remove before F30
>> -Provides: %{name}-python = %{version}-%{release}
>> -Provides: %{name}-python%{?_isa} = %{version}-%{release}
>> -Obsoletes: %{name}-python < %{version}-%{release}
>> +%{?python_provide:%python_provide %{python_package}}
>> %endif # fedora
>>
>> %description -n %{python_package}
>> The %{python_package} package contains a module that permits applications
>> written in the Python programming language to use the interface
>> supplied by the sanlock library.
>>
>> %files -n %{python_package}
>> -%{python2_sitearch}/sanlock_python-*.egg-info
>> -%{python2_sitearch}/sanlock.so
>> +%{python3_sitearch}/sanlock_python-*.egg-info
>> +%{python3_sitearch}/sanlock*.so
>>
>> %package devel
>> Summary: Development files for %{name}
>> Requires: %{name}-lib = %{version}-%{release}
>>
>> @@ -201,8 +190,11 @@ common sanlock lockspace.
>> %dir %attr(-,root,root) %{_rundir}/sanlk-resetd
>> %{_mandir}/man8/sanlk-reset*
>>
>>
>> %changelog
>> +* Wed Jun 12 2019 Nir Soffer <nsoffer(a)redhat.com> - 3.8.0-1
>> +- Convert spec to python 3
>> +
>>
>
> Not sure it's important to add to the changelog section: mixing tabs and
> spaces fix
>
This is internal change that does not affect the behavior so I don't think
there is point
to mention it in the changelog.
>
>
>> * Mon Mar 25 2019 Nir Soffer <nsoffer(a)redhat.com> - 3.7.0-1
>> - Import spec from Fedora master branch (371e11a)
>> - Fix spec to work on CentOS (20efe91)
>> --
>> 2.17.2
>>
>>
4 years, 9 months
[PATCH v2 0/2] tests for clearing resource
by Vojtech Juranek
Add tests for clearing the resource. Also extract reading sanlock
magic number into helper function.
Vojtech Juranek (2):
tests: replace reading of magic number by helper function
tests: add tests for clearing resource
tests/constants.py | 4 +++
tests/python_test.py | 67 +++++++++++++++++++++++++++++++++-----------
tests/util.py | 11 ++++++++
3 files changed, 66 insertions(+), 16 deletions(-)
--
2.20.1
4 years, 9 months
[PATCH 0/2] Fix python style issues
by Nir Soffer
Fix python style issues and add flake8 style check to the build to keep
the code in good shape.
Nir Soffer (2):
build: Add flak8 env
python: Fix style issues
README.dev | 4 ++++
python/example.py | 28 +++++++++++++++++++--------
tests/conftest.py | 8 ++++----
tests/python_test.py | 46 ++++++++++++++++++++++++++++++--------------
tox.ini | 12 +++++++++++-
5 files changed, 71 insertions(+), 27 deletions(-)
--
2.17.2
4 years, 9 months
[PATCH] tests: add tests for clearing resource
by Vojtech Juranek
Add tests for write_resource() with parameter 'clear' set to True, which
shoudl result into clearing the resource (overwritting PAXOS_DISK_MAGIC
with PAXOS_DISK_CLEAR).
---
tests/constants.py | 4 +++
tests/python_test.py | 71 ++++++++++++++++++++++++++++++++++++++++++++
2 files changed, 75 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 3242f57..185c3e1 100644
--- a/tests/python_test.py
+++ b/tests/python_test.py
@@ -234,6 +234,77 @@ def test_write_resource_4k_invalid_sector_size(sanlock_daemon, user_4k_path):
assert e.value.errno == errno.EINVAL
+(a)pytest.mark.parametrize("filename,encoding", FILE_NAMES)
+(a)pytest.mark.parametrize("size,offset", [
+ # Smallest offset.
+ (MIN_RES_SIZE, 0),
+ # Large offset.
+ (LARGE_FILE_SIZE, LARGE_FILE_SIZE - MIN_RES_SIZE),
+])
+def test_clear_resource(tmpdir, sanlock_daemon, filename, encoding, size, offset):
+ path = util.generate_path(tmpdir, filename, encoding)
+ util.create_file(path, size)
+ disks = [(path, offset)]
+
+ # Test write, clear and read with default alignment and sector size values.
+ 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, offset=offset)
+ assert e.value.errno == constants.SANLK_LEADER_MAGIC
+
+ # Test write, clear and read with explicit alignment and sector size
+ # values.
+ sanlock.write_resource(
+ b"ls_name", b"res_name", disks, align=ALIGNMENT_1M,
+ sector=SECTOR_SIZE_512, clear=True)
+
+ with pytest.raises(sanlock.SanlockException) as e:
+ sanlock.read_resource(
+ path, offset=offset, align=ALIGNMENT_1M, sector=SECTOR_SIZE_512)
+ assert e.value.errno == constants.SANLK_LEADER_MAGIC
+
+ with io.open(path, "rb") as f:
+ f.seek(offset)
+ magic, = struct.unpack("< I", f.read(4))
+ assert magic == constants.PAXOS_DISK_CLEAR
+
+ util.check_guard(path, size)
+
+
+(a)pytest.mark.parametrize("align", sanlock.ALIGN_SIZE)
+def test_clear_resource_4k(sanlock_daemon, user_4k_path, align):
+ disks = [(user_4k_path, 0)]
+
+ # Poison resource area, ensuring that previous tests will not break this
+ # test, and sanlock does not write beyond the lockspace area.
+ with io.open(user_4k_path, "rb+") as f:
+ f.write(align * b"x")
+ util.write_guard(user_4k_path, align)
+
+ sanlock.write_resource(
+ b"ls_name", b"res_name", disks, align=align, sector=SECTOR_SIZE_4K)
+ sanlock.write_resource(
+ b"ls_name", b"res_name",
+ disks,
+ align=align,
+ sector=SECTOR_SIZE_4K,
+ clear=True)
+
+ with pytest.raises(sanlock.SanlockException) as e:
+ sanlock.read_resource(user_4k_path, align=align, sector=SECTOR_SIZE_4K)
+ assert e.value.errno == constants.SANLK_LEADER_MAGIC
+
+ # Verify that resource was cleared.
+ with io.open(user_4k_path, "rb") as f:
+ magic, = struct.unpack("< I", f.read(4))
+ assert magic == constants.PAXOS_DISK_CLEAR
+
+ # Check that sanlock did not write beyond the lockspace area.
+ util.check_guard(user_4k_path, align)
+
+
def test_read_resource_4k_invalid_sector_size(sanlock_daemon, user_4k_path):
disks = [(user_4k_path, 0)]
--
2.20.1
4 years, 9 months
[sanlock] 02/02: release 3.8.0
by pagure@pagure.io
This is an automated email from the git hooks/post-receive script.
teigland pushed a commit to branch master
in repository sanlock.
commit 1b159cad7b11a570d6353df089ee48171a8dbc28
Author: David Teigland <teigland(a)redhat.com>
AuthorDate: Wed Jun 12 14:07:38 2019 -0500
release 3.8.0
---
VERSION | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/VERSION b/VERSION
index c1e43e6..1981190 100644
--- a/VERSION
+++ b/VERSION
@@ -1 +1 @@
-3.7.3
+3.8.0
--
To stop receiving notification emails like this one, please contact
the administrator of this repository.
4 years, 9 months
[sanlock] 01/02: spec: Convert development spec to python 3
by pagure@pagure.io
This is an automated email from the git hooks/post-receive script.
teigland pushed a commit to branch master
in repository sanlock.
commit 1fdae8ea67442df78d792f91ac029133ff7ded28
Author: Nir Soffer <nirsof(a)gmail.com>
AuthorDate: Wed Jun 12 22:03:39 2019 +0300
spec: Convert development spec to python 3
The development spec creates now only python 3 packages. This will be
the base for Fedora 30 and CentOS 8.1 spec.
Also make rpmlint happy about mixing tabs and spaces by using only
spaces.
Signed-off-by: Nir Soffer <nsoffer(a)redhat.com>
---
sanlock.spec.in | 42 +++++++++++++++++-------------------------
1 file changed, 17 insertions(+), 25 deletions(-)
diff --git a/sanlock.spec.in b/sanlock.spec.in
index 9ad2985..087076c 100644
--- a/sanlock.spec.in
+++ b/sanlock.spec.in
@@ -18,8 +18,8 @@ BuildRequires: libaio-devel
BuildRequires: libblkid-devel
BuildRequires: libuuid-devel
BuildRequires: make
-BuildRequires: python2
-BuildRequires: python2-devel
+BuildRequires: python3
+BuildRequires: python3-devel
BuildRequires: systemd-units
Requires: %{name}-lib = %{version}-%{release}
Requires(pre): /usr/sbin/groupadd
@@ -30,14 +30,7 @@ Requires(preun): systemd-units
Requires(postun): systemd-units
Source0: https://releases.pagure.org/sanlock/%{name}-%{version}.tar.gz
-# On Fedora we switched sanlock-python to python2-sanlock, as part of python 3
-# porting. This change was not done yet for RHEL.
-# TODO: remove when RHEL switches to pythonN-sanlock.
-%if 0%{?fedora}
-%global python_package python2-%{name}
-%else # centos/rhel
-%global python_package %{name}-python
-%endif
+%global python_package python3-%{name}
%description
The sanlock daemon manages leases for applications on hosts using shared storage.
@@ -53,7 +46,7 @@ The sanlock daemon manages leases for applications on hosts using shared storage
# upstream does not support _smp_mflags
CFLAGS=$RPM_OPT_FLAGS make -C wdmd
CFLAGS=$RPM_OPT_FLAGS make -C src
-CFLAGS=$RPM_OPT_FLAGS make -C python
+CFLAGS=$RPM_OPT_FLAGS make -C python PY_VERSION=3.6
CFLAGS=$RPM_OPT_FLAGS make -C reset
%install
@@ -67,7 +60,7 @@ make -C wdmd \
make -C python \
install LIBDIR=%{_libdir} \
DESTDIR=$RPM_BUILD_ROOT \
- PY_VERSION=2.7 # TODO: fix PY_VERSION to 3.6 when 2020 will arrive
+ PY_VERSION=3.6
make -C reset \
install LIBDIR=%{_libdir} \
DESTDIR=$RPM_BUILD_ROOT
@@ -79,13 +72,13 @@ install -D -m 0644 init.d/wdmd.service.native $RPM_BUILD_ROOT/%{_unitdir}/wdmd.s
install -D -m 0644 init.d/sanlk-resetd.service $RPM_BUILD_ROOT/%{_unitdir}/sanlk-resetd.service
install -D -m 0644 src/logrotate.sanlock \
- $RPM_BUILD_ROOT/etc/logrotate.d/sanlock
+ $RPM_BUILD_ROOT/etc/logrotate.d/sanlock
install -D -m 0644 src/sanlock.conf \
- $RPM_BUILD_ROOT/etc/sanlock/sanlock.conf
+ $RPM_BUILD_ROOT/etc/sanlock/sanlock.conf
install -D -m 0644 init.d/wdmd.sysconfig \
- $RPM_BUILD_ROOT/etc/sysconfig/wdmd
+ $RPM_BUILD_ROOT/etc/sysconfig/wdmd
install -Dd -m 0755 $RPM_BUILD_ROOT/etc/wdmd.d
install -Dd -m 0775 $RPM_BUILD_ROOT/%{_rundir}/sanlock
@@ -93,10 +86,10 @@ install -Dd -m 0775 $RPM_BUILD_ROOT/%{_rundir}/sanlk-resetd
%pre
getent group sanlock > /dev/null || /usr/sbin/groupadd \
- -g 179 sanlock
+ -g 179 sanlock
getent passwd sanlock > /dev/null || /usr/sbin/useradd \
- -u 179 -c "sanlock" -s /sbin/nologin -r \
- -g 179 -d /run/sanlock sanlock
+ -u 179 -c "sanlock" -s /sbin/nologin -r \
+ -g 179 -d /run/sanlock sanlock
/usr/sbin/usermod -a -G disk sanlock
%post
@@ -146,11 +139,7 @@ access to the shared disks.
Summary: Python bindings for the sanlock library
Requires: %{name}-lib = %{version}-%{release}
%if 0%{?fedora}
-%{?python_provide:%python_provide python2-%{name}}
-# Remove before F30
-Provides: %{name}-python = %{version}-%{release}
-Provides: %{name}-python%{?_isa} = %{version}-%{release}
-Obsoletes: %{name}-python < %{version}-%{release}
+%{?python_provide:%python_provide %{python_package}}
%endif # fedora
%description -n %{python_package}
@@ -159,8 +148,8 @@ written in the Python programming language to use the interface
supplied by the sanlock library.
%files -n %{python_package}
-%{python2_sitearch}/sanlock_python-*.egg-info
-%{python2_sitearch}/sanlock.so
+%{python3_sitearch}/sanlock_python-*.egg-info
+%{python3_sitearch}/sanlock*.so
%package devel
Summary: Development files for %{name}
@@ -203,6 +192,9 @@ common sanlock lockspace.
%changelog
+* Wed Jun 12 2019 Nir Soffer <nsoffer(a)redhat.com> - 3.8.0-1
+- Convert spec to python 3
+
* Mon Mar 25 2019 Nir Soffer <nsoffer(a)redhat.com> - 3.7.0-1
- Import spec from Fedora master branch (371e11a)
- Fix spec to work on CentOS (20efe91)
--
To stop receiving notification emails like this one, please contact
the administrator of this repository.
4 years, 9 months