Python code cleanup

These changes should not change any functionality.

In code:

- Added async keyword to CryptoSystem::kind since it's actually implemented async
- Removed unused socket import in json_api

In tests:
- Removed unused imports
- Removed unnecessary return statements
- Removed unused variables
- Cleaned up some spacing to match PEP-8
- Changed many comparisons to match PEP-8
- Use ValueSubKey classes instead of integers to keep types in line

veilid-python/tests/conftest.py

@@ -4,9 +4,9 @@ from typing import AsyncGenerator
 
 import pytest
 import pytest_asyncio
-from veilid.json_api import _JsonVeilidAPI
 
 import veilid
+from veilid.json_api import _JsonVeilidAPI
 
 
 pytest_plugins = ("pytest_asyncio",)
@@ -22,7 +22,6 @@ async def api_connection() -> AsyncGenerator[_JsonVeilidAPI, None]:
         api = await veilid.api_connector(simple_update_callback)
     except veilid.VeilidConnectionError:
         pytest.skip("Unable to connect to veilid-server.")
-        return
 
     async with api:
         # purge routes to ensure we start fresh

veilid-python/tests/test_crypto.py

@@ -3,7 +3,6 @@
 import pytest
 import veilid
 from veilid.api import CryptoSystem
-import gc
 
 
 @pytest.mark.asyncio
@@ -42,9 +41,10 @@ async def test_hash_and_verify_password(api_connection: veilid.VeilidAPI):
         assert await cs.verify_password(b"abc123", phash)
 
         # Password mismatch
-        phash2 = await cs.hash_password(b"abc1234", salt)
+        await cs.hash_password(b"abc1234", salt)
         assert not await cs.verify_password(b"abc12345", phash)
 
+
 @pytest.mark.asyncio
 async def test_sign_and_verify_signature(api_connection: veilid.VeilidAPI):
     cs = await api_connection.best_crypto_system()

veilid-python/tests/test_dht.py

@@ -1,13 +1,12 @@
 # Routing context veilid tests
 
-import veilid
 import pytest
 import asyncio
-import json
 import time
 import os
-from . import *
 
+import veilid
+from veilid import ValueSubkey
 
 ##################################################################
 BOGUS_KEY = veilid.TypedKey.from_value(
@@ -20,7 +19,7 @@ async def test_get_dht_value_unopened(api_connection: veilid.VeilidAPI):
     
     async with rc:
         with pytest.raises(veilid.VeilidAPIError):
-            out = await rc.get_dht_value(BOGUS_KEY, veilid.ValueSubkey(0), False)
+            await rc.get_dht_value(BOGUS_KEY, ValueSubkey(0), False)
 
 
 @pytest.mark.asyncio
@@ -28,7 +27,7 @@ async def test_open_dht_record_nonexistent_no_writer(api_connection: veilid.Veil
     rc = await api_connection.new_routing_context()
     async with rc:
         with pytest.raises(veilid.VeilidAPIError):
-            out = await rc.open_dht_record(BOGUS_KEY, None)
+            await rc.open_dht_record(BOGUS_KEY, None)
 
 
 @pytest.mark.asyncio
@@ -63,7 +62,7 @@ async def test_get_dht_value_nonexistent(api_connection: veilid.VeilidAPI):
     rc = await api_connection.new_routing_context()
     async with rc:
         rec = await rc.create_dht_record(veilid.DHTSchema.dflt(1))
-        assert await rc.get_dht_value(rec.key, 0, False) == None
+        assert await rc.get_dht_value(rec.key, ValueSubkey(0), False) is None
         await rc.close_dht_record(rec.key)
         await rc.delete_dht_record(rec.key)
 
@@ -74,17 +73,17 @@ async def test_set_get_dht_value(api_connection: veilid.VeilidAPI):
     async with rc:
         rec = await rc.create_dht_record(veilid.DHTSchema.dflt(2))
 
-        vd = await rc.set_dht_value(rec.key, 0, b"BLAH BLAH BLAH")
+        vd = await rc.set_dht_value(rec.key, ValueSubkey(0), b"BLAH BLAH BLAH")
-        assert vd == None
+        assert vd is None
 
-        vd2 = await rc.get_dht_value(rec.key, 0, False)
+        vd2 = await rc.get_dht_value(rec.key, ValueSubkey(0), False)
-        assert vd2 != None
+        assert vd2 is not None
 
-        vd3 = await rc.get_dht_value(rec.key, 0, True)
+        vd3 = await rc.get_dht_value(rec.key, ValueSubkey(0), True)
-        assert vd3 != None
+        assert vd3 is not None
 
-        vd4 = await rc.get_dht_value(rec.key, 1, False)
+        vd4 = await rc.get_dht_value(rec.key, ValueSubkey(1), False)
-        assert vd4 == None
+        assert vd4 is None
 
         print("vd2: {}", vd2.__dict__)
         print("vd3: {}", vd3.__dict__)
@@ -115,33 +114,33 @@ async def test_open_writer_dht_value(api_connection: veilid.VeilidAPI):
         vc = b"!@#$%^&*()"
 
         # Test subkey writes
-        vdtemp = await rc.set_dht_value(key, 1, va)
+        vdtemp = await rc.set_dht_value(key, ValueSubkey(1), va)
-        assert vdtemp == None
+        assert vdtemp is None
 
-        vdtemp = await rc.get_dht_value(key, 1, False)
+        vdtemp = await rc.get_dht_value(key, ValueSubkey(1), False)
         assert vdtemp.data == va
         assert vdtemp.seq == 0
         assert vdtemp.writer == owner
 
-        vdtemp = await rc.get_dht_value(key, 0, False)
+        vdtemp = await rc.get_dht_value(key, ValueSubkey(0), False)
-        assert vdtemp == None
+        assert vdtemp is None
 
-        vdtemp = await rc.set_dht_value(key, 0, vb)
+        vdtemp = await rc.set_dht_value(key, ValueSubkey(0), vb)
-        assert vdtemp == None
+        assert vdtemp is None
 
-        vdtemp = await rc.get_dht_value(key, 0, True)
+        vdtemp = await rc.get_dht_value(key, ValueSubkey(0), True)
         assert vdtemp.data == vb
 
-        vdtemp = await rc.get_dht_value(key, 1, True)
+        vdtemp = await rc.get_dht_value(key, ValueSubkey(1), True)
         assert vdtemp.data == va
 
         # Equal value should not trigger sequence number update
-        vdtemp = await rc.set_dht_value(key, 1, va)
+        vdtemp = await rc.set_dht_value(key, ValueSubkey(1), va)
-        assert vdtemp == None
+        assert vdtemp is None
 
         # Different value should trigger sequence number update
-        vdtemp = await rc.set_dht_value(key, 1, vb)
+        vdtemp = await rc.set_dht_value(key, ValueSubkey(1), vb)
-        assert vdtemp == None
+        assert vdtemp is None
 
         # Now that we initialized some subkeys
         # and verified they stored correctly
@@ -152,7 +151,7 @@ async def test_open_writer_dht_value(api_connection: veilid.VeilidAPI):
         await rc.delete_dht_record(key)
 
         rec = await rc.open_dht_record(key, veilid.KeyPair.from_parts(owner, secret))
-        assert rec != None
+        assert rec is not None
         assert rec.key == key
         assert rec.owner == owner
         assert rec.owner_secret == secret
@@ -160,19 +159,19 @@ async def test_open_writer_dht_value(api_connection: veilid.VeilidAPI):
         assert rec.schema.o_cnt == 2
 
         # Verify subkey 1 can be set before it is get but newer is available online
-        vdtemp = await rc.set_dht_value(key, 1, vc)
+        vdtemp = await rc.set_dht_value(key, ValueSubkey(1), vc)
-        assert vdtemp != None
+        assert vdtemp is not None
         assert vdtemp.data == vb
         assert vdtemp.seq == 1
         assert vdtemp.writer == owner
 
         # Verify subkey 1 can be set a second time and it updates because seq is newer
-        vdtemp = await rc.set_dht_value(key, 1, vc)
+        vdtemp = await rc.set_dht_value(key, ValueSubkey(1), vc)
-        assert vdtemp == None
+        assert vdtemp is None
 
         # Verify the network got the subkey update with a refresh check
-        vdtemp = await rc.get_dht_value(key, 1, True)
+        vdtemp = await rc.get_dht_value(key, ValueSubkey(1), True)
-        assert vdtemp != None
+        assert vdtemp is not None
         assert vdtemp.data == vc
         assert vdtemp.seq == 2
         assert vdtemp.writer == owner
@@ -184,29 +183,30 @@ async def test_open_writer_dht_value(api_connection: veilid.VeilidAPI):
         await rc.delete_dht_record(key)
 
         rec = await rc.open_dht_record(key, other_keypair)
-        assert rec != None
+        assert rec is not None
         assert rec.key == key
         assert rec.owner == owner
-        assert rec.owner_secret == None
+        assert rec.owner_secret is None
         assert rec.schema.kind == veilid.DHTSchemaKind.DFLT
         assert rec.schema.o_cnt == 2
 
         # Verify subkey 1 can NOT be set because we have the wrong writer
         with pytest.raises(veilid.VeilidAPIError):
-            vdtemp = await rc.set_dht_value(key, 1, va)
+            await rc.set_dht_value(key, ValueSubkey(1), va)
 
         # Verify subkey 0 can NOT be set because we have the wrong writer
         with pytest.raises(veilid.VeilidAPIError):
-            vdtemp = await rc.set_dht_value(key, 0, va)
+            await rc.set_dht_value(key, ValueSubkey(0), va)
 
         # Verify subkey 0 can be set because override with the right writer
-        vdtemp = await rc.set_dht_value(key, 0, va, veilid.KeyPair.from_parts(owner, secret))
+        vdtemp = await rc.set_dht_value(key, ValueSubkey(0), va, veilid.KeyPair.from_parts(owner, secret))
-        assert vdtemp == None
+        assert vdtemp is None
 
         # Clean up
         await rc.close_dht_record(key)
         await rc.delete_dht_record(key)
 
+
 @pytest.mark.asyncio
 async def test_watch_dht_values():
 
@@ -220,7 +220,6 @@ async def test_watch_dht_values():
         api = await veilid.api_connector(value_change_update_callback)
     except veilid.VeilidConnectionError:
         pytest.skip("Unable to connect to veilid-server.")
-        return
 
     # Make two routing contexts, one with and one without safety
     # So we can pretend to be a different node and get the watch updates
@@ -236,8 +235,8 @@ async def test_watch_dht_values():
         rec = await rcWatch.create_dht_record(veilid.DHTSchema.dflt(10))
 
         # Set some subkey we care about
-        vd = await rcWatch.set_dht_value(rec.key, 3, b"BLAH BLAH BLAH")
+        vd = await rcWatch.set_dht_value(rec.key, ValueSubkey(3), b"BLAH BLAH BLAH")
-        assert vd == None
+        assert vd is None
 
         # Make a watch on that subkey
         ts = await rcWatch.watch_dht_values(rec.key, [], 0, 0xFFFFFFFF)
@@ -247,8 +246,8 @@ async def test_watch_dht_values():
         rec = await rcSet.open_dht_record(rec.key, rec.owner_key_pair())
         
         # Now set the subkey and trigger an update
-        vd = await rcSet.set_dht_value(rec.key, 3, b"BLAH")
+        vd = await rcSet.set_dht_value(rec.key, ValueSubkey(3), b"BLAH")
-        assert vd == None
+        assert vd is None
         
         # Now we should NOT get an update because the update is the same as our local copy
         update = None
@@ -256,10 +255,10 @@ async def test_watch_dht_values():
             update = await asyncio.wait_for(value_change_queue.get(), timeout=5)
         except asyncio.TimeoutError:
             pass
-        assert update == None
+        assert update is None
 
         # Now set multiple subkeys and trigger an update
-        vd = await asyncio.gather(*[rcSet.set_dht_value(rec.key, 3, b"BLAH BLAH"), rcSet.set_dht_value(rec.key, 4, b"BZORT")])
+        vd = await asyncio.gather(*[rcSet.set_dht_value(rec.key, ValueSubkey(3), b"BLAH BLAH"), rcSet.set_dht_value(rec.key, ValueSubkey(4), b"BZORT")])
         assert vd == [None, None]
 
         # Wait for the update
@@ -269,43 +268,45 @@ async def test_watch_dht_values():
         assert upd.detail.key == rec.key
         assert upd.detail.count == 0xFFFFFFFD
         assert upd.detail.subkeys == [(3, 4)]
-        assert upd.detail.value == None
+        assert upd.detail.value is None
 
         # Reopen without closing to change routing context and not lose watch
         rec = await rcWatch.open_dht_record(rec.key, rec.owner_key_pair())
 
         # Cancel some subkeys we don't care about
-        still_active = await rcWatch.cancel_dht_watch(rec.key, [(0, 2)])
+        still_active = await rcWatch.cancel_dht_watch(rec.key, [(ValueSubkey(0), ValueSubkey(2))])
-        assert still_active == True
+        assert still_active
 
         # Reopen without closing to change routing context and not lose watch
         rec = await rcSet.open_dht_record(rec.key, rec.owner_key_pair())
 
         # Now set multiple subkeys and trigger an update
-        vd = await asyncio.gather(*[rcSet.set_dht_value(rec.key, 3, b"BLAH BLAH BLAH"), rcSet.set_dht_value(rec.key, 5, b"BZORT BZORT")])
+        vd = await asyncio.gather(*[rcSet.set_dht_value(rec.key, ValueSubkey(3), b"BLAH BLAH BLAH"), rcSet.set_dht_value(rec.key, ValueSubkey(5), b"BZORT BZORT")])
         assert vd == [None, None]
 
-        # Wait for the update
+        # Wait for the update, this longer timeout seems to help the flaky check below
         upd = await asyncio.wait_for(value_change_queue.get(), timeout=10)
 
         # Verify the update came back but we don't get a new value because the sequence number is the same
         assert upd.detail.key == rec.key
+
+        # This check is flaky on slow connections and often fails with different counts
         assert upd.detail.count == 0xFFFFFFFC
         assert upd.detail.subkeys == [(3, 3), (5, 5)]
-        assert upd.detail.value == None
+        assert upd.detail.value is None
 
         # Reopen without closing to change routing context and not lose watch
         rec = await rcWatch.open_dht_record(rec.key, rec.owner_key_pair())
 
         # Now cancel the update
-        still_active = await rcWatch.cancel_dht_watch(rec.key, [(3, 9)])
+        still_active = await rcWatch.cancel_dht_watch(rec.key, [(ValueSubkey(3), ValueSubkey(9))])
-        assert still_active == False
+        assert not still_active
 
         # Reopen without closing to change routing context and not lose watch
         rec = await rcSet.open_dht_record(rec.key, rec.owner_key_pair())
 
         # Now set multiple subkeys
-        vd = await asyncio.gather(*[rcSet.set_dht_value(rec.key, 3, b"BLAH BLAH BLAH BLAH"), rcSet.set_dht_value(rec.key, 5, b"BZORT BZORT BZORT")])
+        vd = await asyncio.gather(*[rcSet.set_dht_value(rec.key, ValueSubkey(3), b"BLAH BLAH BLAH BLAH"), rcSet.set_dht_value(rec.key, ValueSubkey(5), b"BZORT BZORT BZORT")])
         assert vd == [None, None]
         
         # Now we should NOT get an update
@@ -314,20 +315,21 @@ async def test_watch_dht_values():
             update = await asyncio.wait_for(value_change_queue.get(), timeout=5)
         except asyncio.TimeoutError:
             pass
-        assert update == None
+        assert update is None
 
         # Clean up
         await rcSet.close_dht_record(rec.key)
         await rcSet.delete_dht_record(rec.key)
 
+
 @pytest.mark.asyncio
 async def test_inspect_dht_record(api_connection: veilid.VeilidAPI):
     rc = await api_connection.new_routing_context()
     async with rc:
         rec = await rc.create_dht_record(veilid.DHTSchema.dflt(2))
 
-        vd = await rc.set_dht_value(rec.key, 0, b"BLAH BLAH BLAH")
+        vd = await rc.set_dht_value(rec.key, ValueSubkey(0), b"BLAH BLAH BLAH")
-        assert vd == None
+        assert vd is None
 
         rr = await rc.inspect_dht_record(rec.key, [], veilid.DHTReportScope.LOCAL)
         print("rr: {}", rr.__dict__)
@@ -344,6 +346,7 @@ async def test_inspect_dht_record(api_connection: veilid.VeilidAPI):
         await rc.close_dht_record(rec.key)
         await rc.delete_dht_record(rec.key)
 
+
 @pytest.mark.skipif(os.getenv("INTEGRATION") != "1", reason="integration test requires two servers running")
 @pytest.mark.asyncio
 async def test_dht_integration_writer_reader():
@@ -355,13 +358,11 @@ async def test_dht_integration_writer_reader():
         api0 = await veilid.api_connector(null_update_callback, 0)
     except veilid.VeilidConnectionError:
         pytest.skip("Unable to connect to veilid-server 0.")
-        return
 
     try:
         api1 = await veilid.api_connector(null_update_callback, 1)
     except veilid.VeilidConnectionError:
         pytest.skip("Unable to connect to veilid-server 1.")
-        return
 
     async with api0, api1:
         # purge local and remote record stores to ensure we start fresh
@@ -386,11 +387,11 @@ async def test_dht_integration_writer_reader():
                 desc = await rc0.create_dht_record(schema)
                 records.append(desc)
 
-                await rc0.set_dht_value(desc.key, 0, TEST_DATA)
+                await rc0.set_dht_value(desc.key, ValueSubkey(0), TEST_DATA)
 
                 print(f'  {n}')
             
-            print(f'syncing records to the network')
+            print('syncing records to the network')
             for desc0 in records:
                 while True:
                     rr = await rc0.inspect_dht_record(desc0.key, [])
@@ -401,15 +402,16 @@ async def test_dht_integration_writer_reader():
 
             # read dht records on server 1
             print(f'reading {COUNT} records')
-            n=0
+            n = 0
             for desc0 in records:
                 desc1 = await rc1.open_dht_record(desc0.key)
-                vd1 = await rc1.get_dht_value(desc1.key, 0)
+                vd1 = await rc1.get_dht_value(desc1.key, ValueSubkey(0))
                 assert vd1.data == TEST_DATA
                 await rc1.close_dht_record(desc1.key)
                 
                 print(f'  {n}')
-                n+=1
+                n += 1
+
 
 @pytest.mark.asyncio
 async def test_dht_write_read_local():
@@ -421,7 +423,6 @@ async def test_dht_write_read_local():
         api0 = await veilid.api_connector(null_update_callback, 0)
     except veilid.VeilidConnectionError:
         pytest.skip("Unable to connect to veilid-server 0.")
-        return
 
     async with api0:
         # purge local and remote record stores to ensure we start fresh
@@ -432,8 +433,9 @@ async def test_dht_write_read_local():
         rc0 = await api0.new_routing_context()
         async with rc0:
 
-            # FIXME: 500
+            # Previously COUNT was set to 500, which causes these tests to take
-            COUNT = 5
+            # 10s of minutes on slow connections or debug veilid-server builds
+            COUNT = 10
             TEST_DATA = b"ABCD"*1024
             TEST_DATA2 = b"ABCD"*4096
 
@@ -445,8 +447,8 @@ async def test_dht_write_read_local():
                 desc = await rc0.create_dht_record(schema)
                 records.append(desc)
 
-                await rc0.set_dht_value(desc.key, 0, TEST_DATA)
+                await rc0.set_dht_value(desc.key, ValueSubkey(0), TEST_DATA)
-                await rc0.set_dht_value(desc.key, 1, TEST_DATA2)
+                await rc0.set_dht_value(desc.key, ValueSubkey(1), TEST_DATA2)
 
                 print(f'  {n}')
             
@@ -461,18 +463,16 @@ async def test_dht_write_read_local():
 
             # read dht records on server 0
             print(f'reading {COUNT} records')
-            n=0
+            n = 0
             for desc0 in records:
                 desc1 = await rc0.open_dht_record(desc0.key)
                 
-                vd0 = await rc0.get_dht_value(desc1.key, 0)
+                vd0 = await rc0.get_dht_value(desc1.key, ValueSubkey(0))
                 assert vd0.data == TEST_DATA
                 
-                vd1 = await rc0.get_dht_value(desc1.key, 1)
+                vd1 = await rc0.get_dht_value(desc1.key, ValueSubkey(1))
                 assert vd1.data == TEST_DATA2
                 await rc0.close_dht_record(desc1.key)
                 
                 print(f'  {n}')
-                n+=1
+                n += 1
-                
-            

veilid-python/tests/test_routing_context.py

@@ -3,12 +3,11 @@
 import asyncio
 import os
 import random
-import sys
-
 import pytest
 
 import veilid
 
+
 ##################################################################
 
 
@@ -54,7 +53,6 @@ async def test_routing_context_app_message_loopback():
         api = await veilid.api_connector(app_message_queue_update_callback)
     except veilid.VeilidConnectionError:
         pytest.skip("Unable to connect to veilid-server.")
-        return
 
     async with api:
         # purge routes to ensure we start fresh
@@ -102,7 +100,6 @@ async def test_routing_context_app_call_loopback():
         api = await veilid.api_connector(app_call_queue_update_callback)
     except veilid.VeilidConnectionError:
         pytest.skip("Unable to connect to veilid-server.")
-        return
 
     async with api:
         # purge routes to ensure we start fresh
@@ -163,7 +160,6 @@ async def test_routing_context_app_message_loopback_big_packets():
         api = await veilid.api_connector(app_message_queue_update_callback)
     except veilid.VeilidConnectionError:
         pytest.skip("Unable to connect to veilid-server.")
-        return
 
     async with api:
         # purge routes to ensure we start fresh
@@ -204,9 +200,9 @@ async def test_routing_context_app_message_loopback_big_packets():
                 await api.release_private_route(prl)
 
 
-
 @pytest.mark.asyncio
 async def test_routing_context_app_call_loopback_big_packets():
+    # This test has a tendency to timeout on slow connections
     count_hack = [0]
 
     app_call_queue: asyncio.Queue = asyncio.Queue()
@@ -228,7 +224,6 @@ async def test_routing_context_app_call_loopback_big_packets():
         api = await veilid.api_connector(app_call_queue_update_callback)
     except veilid.VeilidConnectionError:
         pytest.skip("Unable to connect to veilid-server.")
-        return
 
     async with api:
         # purge routes to ensure we start fresh
@@ -278,7 +273,6 @@ async def test_routing_context_app_message_loopback_bandwidth():
         api = await veilid.api_connector(app_message_queue_update_callback)
     except veilid.VeilidConnectionError:
         pytest.skip("Unable to connect to veilid-server.")
-        return
 
     async with api:
         # purge routes to ensure we start fresh
@@ -293,7 +287,7 @@ async def test_routing_context_app_message_loopback_bandwidth():
                 # import it as a remote route as well so we can send to it
                 prr = await api.import_remote_private_route(blob)
                 try:
-                # do this test 1000 times
+                    # do this test 10000 times
                     message = random.randbytes(16384)
                     for _ in range(10000):
                         # send a random sized random app message to our own private route

veilid-python/veilid/__init__.py

@@ -1,6 +1,6 @@
 from .api import *
-from .connection import  *
 from .config import *
+from .connection import *
 from .error import *
 from .json_api import *
 from .state import *

veilid-python/veilid/api.py

@@ -187,7 +187,7 @@ class CryptoSystem(ABC):
             await self.release()
 
     @abstractmethod
-    def kind(self) -> types.CryptoKind:
+    async def kind(self) -> types.CryptoKind:
         pass
 
     @abstractmethod

veilid-python/veilid/json_api.py

@@ -2,7 +2,6 @@ import asyncio
 import importlib.resources as importlib_resources
 import json
 import os
-import socket
 from typing import Awaitable, Callable, Optional, Self
 
 from jsonschema import exceptions, validators