Merge remote-tracking branch 'origin/topic/bbannier/modernize-project'

.pre-commit-config.yaml

@@ -1,19 +1,13 @@
 repos:
-- repo: https://github.com/psf/black
-  rev: 23.7.0
+- repo: https://github.com/astral-sh/ruff-pre-commit
+  rev: v0.5.1
   hooks:
-  - id: black
-
-- repo: https://github.com/PyCQA/pylint
-  rev: v3.0.0a7
-  hooks:
-  - id: pylint
-    additional_dependencies:
-      - argparse_manpage
-      - websocket-client
+  - id: ruff
+    args: [--fix]
+  - id: ruff-format
 
 - repo: https://github.com/pre-commit/pre-commit-hooks
-  rev: v4.4.0
+  rev: v4.6.0
   hooks:
   - id: trailing-whitespace
   - id: end-of-file-fixer

CHANGES

@@ -1,3 +1,47 @@
+1.3.1-36 | 2024-07-23 10:08:29 +0200
+
+  * Bump pre-commit hooks (Benjamin Bannier, Corelight)
+
+  * Stop linting with pylint (Benjamin Bannier, Corelight)
+
+  * Fix ruff N lints (Benjamin Bannier, Corelight)
+
+  * Fix ruff C4 lints (Benjamin Bannier, Corelight)
+
+  * Fix ruff B lints (Benjamin Bannier, Corelight)
+
+  * Fix ruff I lints (Benjamin Bannier, Corelight)
+
+  * Fix ruff COM lints (Benjamin Bannier, Corelight)
+
+  * Start linting with ruff (Benjamin Bannier, Corelight)
+
+  * Format project with ruff-format instead of black (Benjamin Bannier, Corelight)
+
+  * Fix useless return flagged by linters (Benjamin Bannier, Corelight)
+
+  * Clean up imports (Benjamin Bannier, Corelight)
+
+  * Make optional dependencies required (Benjamin Bannier, Corelight)
+
+    We would previously included code to deal with optional dependencies in
+    an ad hoc way in the code. Since we are a proper package which already
+    has requirements this is not really needded. Instead simply require
+    these dependencies with this patch.
+
+  * Switch coverage generation over to `pytest-cov` (Benjamin Bannier, Corelight)
+
+  * Remove workarounds for running from uninstalled package (Benjamin Bannier, Corelight)
+
+    We had dedicated code which replicated setuptools' "development
+    mode"[^1] in that we could import from a not installed package. This not
+    only complicated the code for no good reason, but also tripped up
+    linting tools.
+
+    This patch removes these workarounds.
+
+    [^1]: https://setuptools.pypa.io/en/latest/userguide/development_mode.html
+
 1.3.1-21 | 2024-07-08 22:24:21 -0700
 
   * Move project configuration to `pyproject.toml` (Benjamin Bannier, Corelight)

Makefile

@@ -7,16 +7,7 @@ test:
 
 .PHONY: test-coverage
 test-coverage:
-	command -v coverage >/dev/null || { echo "Please install coverage via 'pip install coverage'"; exit 1; }
-	cd tests \
-	    && coverage run --source=../zeekclient -m unittest \
-	       test_brokertypes.py \
-	       test_cli.py \
-	       test_config_io.py \
-	       test_config_overrides.py \
-	       test_controller.py \
-	       test_types.py \
-	    && coverage report -m
+	pytest --cov=zeekclient  --cov-report=html --cov-report=term
 
 .PHONY: man
 man:

VERSION

@@ -1 +1 @@
-1.3.1-21
+1.3.1-36

man/build.py

@@ -5,11 +5,7 @@
 import os
 import sys
 
-try:
-    from argparse_manpage.manpage import Manpage
-except ImportError:
-    print("The manpage builder needs the argparse_manpage package.")
-    sys.exit(1)
+from argparse_manpage.manpage import Manpage
 
 LOCALDIR = os.path.dirname(os.path.realpath(__file__))
 ROOTDIR = os.path.normpath(os.path.join(LOCALDIR, ".."))
@@ -18,7 +14,7 @@
 # zeekclient package locally.
 sys.path.insert(0, ROOTDIR)
 
-import zeekclient.cli  # pylint: disable=wrong-import-position,import-error
+import zeekclient.cli  # noqa: E402
 
 
 def main():

pyproject.toml

@@ -4,6 +4,8 @@ dynamic = ["version"]
 description = "A CLI for Zeek's Management Framework"
 dependencies = [
     "websocket-client>=1.8.0",
+    "argcomplete>=3.4.0",
+    "argparse>=1.4.0",
 ]
 readme = "README.md"
 requires-python = ">= 3.8"
@@ -37,6 +39,7 @@ Repository = "https://github.com/zeek/zeek-client"
 [project.optional-dependencies]
 dev = [
     "pytest>=8.1.1",
+    "pytest-cov>=5.0.0",
 ]
 
 [build-system]
@@ -45,3 +48,20 @@ requires = ["setuptools"]
 [tool.setuptools]
 packages = ["zeekclient"]
 script-files = ["zeek-client"]
+
+[tool.ruff.lint]
+select = [
+    "A",
+    "B",
+    "C4",
+    "COM",
+    "F",
+    "I",
+    "ISC",
+    "N",
+    "UP",
+]
+ignore = [
+    "COM812",
+    "ISC001",
+]

tests/test_brokertypes.py

@@ -1,27 +1,43 @@
 """This verifies the behavior of the types provied by the brokertypes module."""
+
 import datetime
-import os.path
-import sys
 import unittest
 
-TESTS = os.path.dirname(os.path.realpath(__file__))
-ROOT = os.path.normpath(os.path.join(TESTS, ".."))
-
-# Prepend the tree's root folder to the module searchpath so we find zeekclient
-# via it. This allows tests to run without package installation.
-sys.path.insert(0, ROOT)
-
-from zeekclient.brokertypes import *  # pylint: disable=wrong-import-position,unused-wildcard-import,wildcard-import
+from zeekclient.brokertypes import (
+    Address,
+    Boolean,
+    Count,
+    DataMessage,
+    Enum,
+    ErrorMessage,
+    HandshakeAckMessage,
+    HandshakeMessage,
+    Integer,
+    NoneType,
+    Port,
+    Real,
+    Set,
+    String,
+    Subnet,
+    Table,
+    Timespan,
+    Timestamp,
+    Vector,
+    ZeekEvent,
+    from_py,
+    ipaddress,
+    unserialize,
+)
 
 
 class TestBrokertypes(unittest.TestCase):
-    def assertEqualRoundtrip(self, data):
+    def assertEqualRoundtrip(self, data):  # noqa: N802
         # This verifies for the given Brokertype object that it can serialize
         # into Broker's wire format, unserialize, and yield an identical object.
         output = type(data).unserialize(data.serialize())
         self.assertEqual(data, output)
 
-    def assertHash(self, val):
+    def assertHash(self, val):  # noqa: N802
         d = {val: 1}
         self.assertEqual(d[val], 1)
 
@@ -110,10 +126,12 @@ def test_timespan(self):
         self.assertEqual(Timespan("10s").to_py(), datetime.timedelta(seconds=10))
 
         self.assertEqual(
-            Timespan("10.5d").to_py(), datetime.timedelta(days=10, hours=12)
+            Timespan("10.5d").to_py(),
+            datetime.timedelta(days=10, hours=12),
         )
         self.assertEqual(
-            Timespan(datetime.timedelta(microseconds=1)), Timespan("1000ns")
+            Timespan(datetime.timedelta(microseconds=1)),
+            Timespan("1000ns"),
         )
         self.assertEqual(Timespan(datetime.timedelta(milliseconds=1)), Timespan("1ms"))
         self.assertEqual(Timespan(datetime.timedelta(seconds=1)), Timespan("1000ms"))
@@ -145,7 +163,8 @@ def test_timestamp(self):
         self.assertEqual(Timestamp(ts1), Timestamp(ts1))
         self.assertEqual(Timestamp(ts1).to_py(), datetime.datetime.fromisoformat(ts1))
         self.assertEqual(
-            Timestamp(ts1), Timestamp(datetime.datetime.fromisoformat(ts1))
+            Timestamp(ts1),
+            Timestamp(datetime.datetime.fromisoformat(ts1)),
         )
 
         self.assertNotEqual(Timestamp(ts1), Timestamp(ts2))
@@ -189,16 +208,20 @@ def test_enum(self):
     def test_address(self):
         self.assertEqual(Address("127.0.0.1"), Address("127.0.0.1"))
         self.assertEqual(
-            Address(ipaddress.ip_address("127.0.0.1")), Address("127.0.0.1")
+            Address(ipaddress.ip_address("127.0.0.1")),
+            Address("127.0.0.1"),
         )
         self.assertEqual(
-            Address(ipaddress.ip_address("2001:db8::")), Address("2001:db8::")
+            Address(ipaddress.ip_address("2001:db8::")),
+            Address("2001:db8::"),
         )
         self.assertEqual(
-            Address("127.0.0.1").to_py(), ipaddress.ip_address("127.0.0.1")
+            Address("127.0.0.1").to_py(),
+            ipaddress.ip_address("127.0.0.1"),
         )
         self.assertEqual(
-            Address("127.0.0.1"), from_py(ipaddress.ip_address("127.0.0.1"))
+            Address("127.0.0.1"),
+            from_py(ipaddress.ip_address("127.0.0.1")),
         )
 
         self.assertNotEqual(Address("127.0.0.1"), Address("10.0.0.1"))
@@ -216,16 +239,20 @@ def test_address(self):
     def test_subnet(self):
         self.assertEqual(Subnet("10.0.0.0/8"), Subnet("10.0.0.0/8"))
         self.assertEqual(
-            Subnet(ipaddress.ip_network("10.0.0.0/8")), Subnet("10.0.0.0/8")
+            Subnet(ipaddress.ip_network("10.0.0.0/8")),
+            Subnet("10.0.0.0/8"),
         )
         self.assertEqual(
-            Subnet(ipaddress.ip_network("2001:db8::/32")), Subnet("2001:db8::/32")
+            Subnet(ipaddress.ip_network("2001:db8::/32")),
+            Subnet("2001:db8::/32"),
         )
         self.assertEqual(
-            Subnet("10.0.0.0/8").to_py(), ipaddress.ip_network("10.0.0.0/8")
+            Subnet("10.0.0.0/8").to_py(),
+            ipaddress.ip_network("10.0.0.0/8"),
         )
         self.assertEqual(
-            Subnet("10.0.0.0/8"), from_py(ipaddress.ip_network("10.0.0.0/8"))
+            Subnet("10.0.0.0/8"),
+            from_py(ipaddress.ip_network("10.0.0.0/8")),
         )
 
         self.assertNotEqual(Subnet("10.0.0.0/8"), Subnet("10.0.0.1"))
@@ -279,7 +306,8 @@ def test_vector(self):
 
         self.assertNotEqual(Vector([from_py(1), from_py("foo")]), Vector([from_py(1)]))
         self.assertNotEqual(
-            Vector([from_py(1), from_py("foo")]), Vector([from_py(1), from_py("noo")])
+            Vector([from_py(1), from_py("foo")]),
+            Vector([from_py(1), from_py("noo")]),
         )
 
         self.assertEqualRoundtrip(Vector([from_py(1), from_py("foo"), from_py(True)]))
@@ -306,7 +334,8 @@ def test_set(self):
 
         self.assertNotEqual(Set({from_py(1), from_py("foo")}), Set({from_py(1)}))
         self.assertNotEqual(
-            Set({from_py(1), from_py("foo")}), Set({from_py(1), from_py("noo")})
+            Set({from_py(1), from_py("foo")}),
+            Set({from_py(1), from_py("noo")}),
         )
 
         self.assertEqualRoundtrip(Set({from_py(1), from_py("foo"), from_py(True)}))
@@ -344,7 +373,7 @@ def test_table(self):
         )
 
         self.assertEqualRoundtrip(
-            Table({from_py("foo"): from_py(1), from_py("bar"): from_py(2)})
+            Table({from_py("foo"): from_py(1), from_py("bar"): from_py(2)}),
         )
         for _ in val:
             pass
@@ -358,22 +387,22 @@ def test_table(self):
         self.assertTrue(String("foo") in val)
 
         self.assertFalse(
-            Table({from_py("foo"): from_py(1)}) < Table({from_py("foo"): from_py(1)})
+            Table({from_py("foo"): from_py(1)}) < Table({from_py("foo"): from_py(1)}),
         )
         self.assertTrue(
-            Table({from_py("foo"): from_py(1)}) < Table({from_py("foo"): from_py(2)})
+            Table({from_py("foo"): from_py(1)}) < Table({from_py("foo"): from_py(2)}),
         )
         self.assertTrue(
             Table({from_py("bar"): from_py(1)})
-            < Table({from_py("foo"): from_py(1), from_py("bar"): from_py(1)})
+            < Table({from_py("foo"): from_py(1), from_py("bar"): from_py(1)}),
         )
         self.assertTrue(
             Table({from_py("foo"): from_py(1)})
-            < Table({from_py("foo"): from_py(1), from_py("bar"): from_py(2)})
+            < Table({from_py("foo"): from_py(1), from_py("bar"): from_py(2)}),
         )
         self.assertTrue(
             Table({from_py("aaa"): from_py(1)})
-            < Table({from_py("foo"): from_py(1), from_py("bar"): from_py(2)})
+            < Table({from_py("foo"): from_py(1), from_py("bar"): from_py(2)}),
         )
         self.assertHash(val)
 
@@ -438,26 +467,31 @@ def test_zeek_event_from_vector_invalid(self):
 
     def test_handshake_message(self):
         self.assertEqual(
-            HandshakeMessage(["foo", "bar"]), HandshakeMessage(["foo", String("bar")])
+            HandshakeMessage(["foo", "bar"]),
+            HandshakeMessage(["foo", String("bar")]),
         )
         self.assertNotEqual(HandshakeMessage(["foo", "bar"]), HandshakeMessage(["foo"]))
         self.assertEqualRoundtrip(HandshakeMessage(["foo", "bar"]))
 
     def test_handshake_ack_message(self):
         self.assertEqual(
-            HandshakeAckMessage("aaaa", "1.0"), HandshakeAckMessage("aaaa", "1.0")
+            HandshakeAckMessage("aaaa", "1.0"),
+            HandshakeAckMessage("aaaa", "1.0"),
         )
         self.assertNotEqual(
-            HandshakeAckMessage("aaaa", "1.0"), HandshakeAckMessage("bbbb", "1.0")
+            HandshakeAckMessage("aaaa", "1.0"),
+            HandshakeAckMessage("bbbb", "1.0"),
         )
         self.assertEqualRoundtrip(HandshakeAckMessage("aaaa", "1.0"))
 
     def test_data_message(self):
         self.assertEqual(
-            DataMessage("foo", String("test")), DataMessage("foo", String("test"))
+            DataMessage("foo", String("test")),
+            DataMessage("foo", String("test")),
         )
         self.assertNotEqual(
-            DataMessage("foo", String("test")), DataMessage("foo", String("other"))
+            DataMessage("foo", String("test")),
+            DataMessage("foo", String("other")),
         )
         self.assertEqualRoundtrip(DataMessage("foo", String("test")))
 
@@ -529,8 +563,8 @@ def test_container_from_broker(self):
                             "@data-type": "integer",
                             "data": "42",
                         },
-                    }
-                ]
-            }
+                    },
+                ],
+            },
         )
         self.assertEqual(1, len(t))