libnmstate: Enforce keyword args for APIs

apply(), commit(), rollback(), show() now accept only keyword args.
testcases are updated to expect errors instead of warnings
deprecation warning code is removed

Signed-off-by: Adwait Thattey <coderdude1999@gmail.com>

libnmstate/deprecation.py

@@ -1,42 +0,0 @@
-#
-# Copyright (c) 2020 Red Hat, Inc.
-#
-# This file is part of nmstate
-#
-# This program is free software: you can redistribute it and/or modify
-# it under the terms of the GNU Lesser General Public License as published by
-# the Free Software Foundation, either version 2.1 of the License, or
-# (at your option) any later version.
-#
-# This program is distributed in the hope that it will be useful,
-# but WITHOUT ANY WARRANTY; without even the implied warranty of
-# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
-# GNU Lesser General Public License for more details.
-#
-# You should have received a copy of the GNU Lesser General Public License
-# along with this program. If not, see <https://www.gnu.org/licenses/>.
-#
-
-import inspect
-import warnings
-
-
-def _warn_keyword_as_positional(func):
-    signature = inspect.signature(func)
-    parameters = signature.parameters.values()
-    intended_posargs_count = len(
-        [p for p in parameters if p.default == inspect.Parameter.empty]
-    )
-
-    def wrapper(*args, **kwargs):
-        for arg_number in range(intended_posargs_count, len(args)):
-            bad_argname = list(parameters)[arg_number].name
-            warnings.warn(
-                f"Specifying '{bad_argname}' as positional argument is "
-                "deprecated. Please specify it as a keyword argument.",
-                FutureWarning,
-                stacklevel=2,
-            )
-        return func(*args, **kwargs)
-
-    return wrapper

libnmstate/netapplier.py

@@ -28,7 +28,6 @@ from libnmstate import nm
 from libnmstate.nm.nmclient import nmclient_context
 from libnmstate import state
 from libnmstate import validator
-from libnmstate.deprecation import _warn_keyword_as_positional
 from libnmstate.error import NmstateConflictError
 from libnmstate.error import NmstateError
 from libnmstate.error import NmstateLibnmError
@@ -42,9 +41,10 @@ VERIFY_RETRY_INTERNAL = 1
 VERIFY_RETRY_TIMEOUT = 5
 
 
-@_warn_keyword_as_positional
 @nmclient_context
-def apply(desired_state, verify_change=True, commit=True, rollback_timeout=60):
+def apply(
+    desired_state, *, verify_change=True, commit=True, rollback_timeout=60
+):
     """
     Apply the desired state
 
@@ -75,8 +75,7 @@ def apply(desired_state, verify_change=True, commit=True, rollback_timeout=60):
         return str(checkpoint.dbuspath)
 
 
-@_warn_keyword_as_positional
-def commit(checkpoint=None):
+def commit(*, checkpoint=None):
     """
     Commit a checkpoint that was received from `apply()`.
 
@@ -92,8 +91,7 @@ def commit(checkpoint=None):
         raise NmstateValueError(str(e))
 
 
-@_warn_keyword_as_positional
-def rollback(checkpoint=None):
+def rollback(*, checkpoint=None):
     """
     Roll back a checkpoint that was received from `apply()`.
 

libnmstate/netinfo.py

@@ -21,7 +21,6 @@ from operator import itemgetter
 from libnmstate import nm
 from libnmstate import validator
 from libnmstate.appliers.ovs_bridge import is_ovs_running
-from libnmstate.deprecation import _warn_keyword_as_positional
 from libnmstate.nm import dns as nm_dns
 from libnmstate.nm.nmclient import nmclient_context
 from libnmstate.schema import Constants
@@ -31,9 +30,8 @@ from libnmstate.schema import Route
 from libnmstate.schema import RouteRule
 
 
-@_warn_keyword_as_positional
 @nmclient_context
-def show(include_status_data=False):
+def show(*, include_status_data=False):
     """
     Reports configuration and status data on the system.
     Configuration data is the set of writable data which can change the system

tests/lib/netapplier_test.py

@@ -181,35 +181,29 @@ def test_edit_existing_bond(netinfo_nm_mock, netapplier_nm_mock):
 
 
 @mock.patch.object(netapplier, "_apply_ifaces_state", lambda *_: None)
-def test_warning_apply():
-    with pytest.warns(FutureWarning) as record:
+def test_error_apply():
+    with pytest.raises(TypeError):
+        # pylint: disable=too-many-function-args
         netapplier.apply({"interfaces": []}, True)
+        # pylint: enable=too-many-function-args
 
-    assert len(record) == 1
-    assert "'verify_change'" in record[0].message.args[0]
-
-    with pytest.warns(FutureWarning) as record:
+    with pytest.raises(TypeError):
+        # pylint: disable=too-many-function-args
         netapplier.apply({"interfaces": []}, True, True, 0)
-
-    assert len(record) == 3
-    assert "'verify_change'" in record[0].message.args[0]
-    assert "'commit'" in record[1].message.args[0]
-    assert "'rollback_timeout'" in record[2].message.args[0]
+        # pylint: enable=too-many-function-args
 
 
 @mock.patch.object(netapplier, "_choose_checkpoint", lambda *_: mock.Mock())
-def test_warning_commit():
-    with pytest.warns(FutureWarning) as record:
+def test_error_commit():
+    with pytest.raises(TypeError):
+        # pylint: disable=too-many-function-args
         netapplier.commit(None)
-
-    assert len(record) == 1
-    assert "'checkpoint'" in record[0].message.args[0]
+        # pylint: enable=too-many-function-args
 
 
 @mock.patch.object(netapplier, "_choose_checkpoint", lambda *_: mock.Mock())
-def test_warning_rollback():
-    with pytest.warns(FutureWarning) as record:
+def test_error_rollback():
+    with pytest.raises(TypeError):
+        # pylint: disable=too-many-function-args
         netapplier.rollback(None)
-
-    assert len(record) == 1
-    assert "'checkpoint'" in record[0].message.args[0]
+        # pylint: enable=too-many-function-args

tests/lib/netinfo_test.py

@@ -137,7 +137,7 @@ def test_netinfo_show_bond_iface(nm_mock, nm_dns_mock):
     assert current_config == report
 
 
-def test_warning_show(nm_mock, nm_dns_mock):
+def test_error_show(nm_mock, nm_dns_mock):
     current_config = {
         DNS.KEY: {DNS.RUNNING: {}, DNS.CONFIG: {}},
         ROUTES: {"config": [], "running": []},
@@ -168,8 +168,7 @@ def test_warning_show(nm_mock, nm_dns_mock):
     nm_dns_mock.get_running.return_value = current_config[DNS.KEY][DNS.RUNNING]
     nm_dns_mock.get_config.return_value = current_config[DNS.KEY][DNS.CONFIG]
 
-    with pytest.warns(FutureWarning) as record:
+    with pytest.raises(TypeError):
+        # pylint: disable=too-many-function-args
         netinfo.show(None)
-
-    assert len(record) == 1
-    assert "'include_status_data'" in record[0].message.args[0]
+        # pylint: enable=too-many-function-args