From c92e07c88e9d1e1e714114b7cfdaa42dd9579dbf Mon Sep 17 00:00:00 2001 From: DumontIO Date: Tue, 13 Aug 2024 16:47:06 -0400 Subject: [PATCH] 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 | 3 +- veilid-python/tests/test_crypto.py | 4 +- veilid-python/tests/test_dht.py | 158 ++++++++++---------- veilid-python/tests/test_routing_context.py | 12 +- veilid-python/veilid/__init__.py | 2 +- veilid-python/veilid/api.py | 2 +- veilid-python/veilid/json_api.py | 1 - 7 files changed, 87 insertions(+), 95 deletions(-) diff --git a/veilid-python/tests/conftest.py b/veilid-python/tests/conftest.py index 46d2f684..875c2df4 100644 --- a/veilid-python/tests/conftest.py +++ b/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 diff --git a/veilid-python/tests/test_crypto.py b/veilid-python/tests/test_crypto.py index 606a9e2c..c093642d 100644 --- a/veilid-python/tests/test_crypto.py +++ b/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() diff --git a/veilid-python/tests/test_dht.py b/veilid-python/tests/test_dht.py index 749b3c09..95055e18 100644 --- a/veilid-python/tests/test_dht.py +++ b/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") - assert vd == None + vd = await rc.set_dht_value(rec.key, ValueSubkey(0), b"BLAH BLAH BLAH") + assert vd is None - vd2 = await rc.get_dht_value(rec.key, 0, False) - assert vd2 != None + vd2 = await rc.get_dht_value(rec.key, ValueSubkey(0), False) + assert vd2 is not None - vd3 = await rc.get_dht_value(rec.key, 0, True) - assert vd3 != None + vd3 = await rc.get_dht_value(rec.key, ValueSubkey(0), True) + assert vd3 is not None - vd4 = await rc.get_dht_value(rec.key, 1, False) - assert vd4 == None + vd4 = await rc.get_dht_value(rec.key, ValueSubkey(1), False) + 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) - assert vdtemp == None + vdtemp = await rc.set_dht_value(key, ValueSubkey(1), va) + 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) - assert vdtemp == None + vdtemp = await rc.get_dht_value(key, ValueSubkey(0), False) + assert vdtemp is None - vdtemp = await rc.set_dht_value(key, 0, vb) - assert vdtemp == None + vdtemp = await rc.set_dht_value(key, ValueSubkey(0), vb) + 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) - assert vdtemp == None + vdtemp = await rc.set_dht_value(key, ValueSubkey(1), va) + assert vdtemp is None # Different value should trigger sequence number update - vdtemp = await rc.set_dht_value(key, 1, vb) - assert vdtemp == None + vdtemp = await rc.set_dht_value(key, ValueSubkey(1), vb) + 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) - assert vdtemp != None + vdtemp = await rc.set_dht_value(key, ValueSubkey(1), vc) + 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) - assert vdtemp == None + vdtemp = await rc.set_dht_value(key, ValueSubkey(1), vc) + assert vdtemp is None # Verify the network got the subkey update with a refresh check - vdtemp = await rc.get_dht_value(key, 1, True) - assert vdtemp != None + vdtemp = await rc.get_dht_value(key, ValueSubkey(1), True) + 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)) - assert vdtemp == None + vdtemp = await rc.set_dht_value(key, ValueSubkey(0), va, veilid.KeyPair.from_parts(owner, secret)) + 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") - assert vd == None + vd = await rcWatch.set_dht_value(rec.key, ValueSubkey(3), b"BLAH BLAH BLAH") + 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") - assert vd == None + vd = await rcSet.set_dht_value(rec.key, ValueSubkey(3), b"BLAH") + 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)]) - assert still_active == True + still_active = await rcWatch.cancel_dht_watch(rec.key, [(ValueSubkey(0), ValueSubkey(2))]) + 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)]) - assert still_active == False + still_active = await rcWatch.cancel_dht_watch(rec.key, [(ValueSubkey(3), ValueSubkey(9))]) + 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") - assert vd == None + vd = await rc.set_dht_value(rec.key, ValueSubkey(0), b"BLAH BLAH BLAH") + 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,27 +402,27 @@ 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(): async def null_update_callback(update: veilid.VeilidUpdate): - pass + pass try: 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 - COUNT = 5 + # Previously COUNT was set to 500, which causes these tests to take + # 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, 1, TEST_DATA2) + await rc0.set_dht_value(desc.key, ValueSubkey(0), TEST_DATA) + 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 - - \ No newline at end of file + n += 1 diff --git a/veilid-python/tests/test_routing_context.py b/veilid-python/tests/test_routing_context.py index b0af957f..ae8a0a3e 100644 --- a/veilid-python/tests/test_routing_context.py +++ b/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 diff --git a/veilid-python/veilid/__init__.py b/veilid-python/veilid/__init__.py index 830c810b..5f8ce664 100644 --- a/veilid-python/veilid/__init__.py +++ b/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 * diff --git a/veilid-python/veilid/api.py b/veilid-python/veilid/api.py index d7c25be9..5bca553d 100644 --- a/veilid-python/veilid/api.py +++ b/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 diff --git a/veilid-python/veilid/json_api.py b/veilid-python/veilid/json_api.py index f79d18e7..fb123b37 100644 --- a/veilid-python/veilid/json_api.py +++ b/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