diff --git a/.github/workflows/lint.yml b/.github/workflows/lint.yml new file mode 100644 index 0000000..6ba65c8 --- /dev/null +++ b/.github/workflows/lint.yml @@ -0,0 +1,23 @@ +name: lint + +on: + push: + branches: [main] + pull_request: + workflow_dispatch: + +jobs: + ruff: + runs-on: ubuntu-latest + steps: + - uses: actions/checkout@v4 + + - uses: actions/setup-python@v5 + with: + python-version: "3.12" + + # Pinned deliberately: an unpinned ruff picks up new rules on release and + # turns a green branch red without anything in this repo changing. + - run: pip install ruff==0.16.2 + + - run: ruff check . diff --git a/config-merger/script/merge_config.py b/config-merger/script/merge_config.py index 0d66d07..387611b 100755 --- a/config-merger/script/merge_config.py +++ b/config-merger/script/merge_config.py @@ -25,10 +25,11 @@ - retina-tracker.yaml: Config overrides for the retina-tracker sidecar """ -import yaml import os -import sys import shutil +import sys + +import yaml from mergedeep import merge @@ -36,7 +37,7 @@ def get_node_id_from_mender(): """Read node_id from Mender device identity file (generated by mender-device-identity)""" node_id_file = '/data/mender/node_id' try: - with open(node_id_file, 'r') as f: + with open(node_id_file) as f: node_id = f.read().strip() if node_id: return node_id @@ -151,7 +152,7 @@ def ensure_node_id(user_config_path): """Add/update node_id in user config from Mender device identity""" try: # Load current user config - with open(user_config_path, 'r') as f: + with open(user_config_path) as f: user_config = yaml.safe_load(f) or {} # Read node_id from Mender device identity (generated by mender-device-identity script) @@ -200,26 +201,26 @@ def main(): defaults_dir = sys.argv[1] user_config_path = sys.argv[2] output_config_path = sys.argv[3] - + # Construct paths default_config = os.path.join(defaults_dir, 'default.yml') forced_config = os.path.join(defaults_dir, 'forced.yml') - + try: # Load default config (baked into container) print(f"Loading default config from {default_config}") - with open(default_config, 'r') as f: + with open(default_config) as f: config = yaml.safe_load(f) - + if not config: print("ERROR: Default config is empty or invalid") sys.exit(1) - + # Create user config from defaults if it doesn't exist (atomic) if not os.path.exists(user_config_path): - print(f"User config not found, copying from defaults...") + print("User config not found, copying from defaults...") os.makedirs(os.path.dirname(user_config_path), exist_ok=True) - + # Write to temp file first, then atomic rename temp_path = user_config_path + '.tmp.' + str(os.getpid()) shutil.copy(default_config, temp_path) @@ -230,33 +231,33 @@ def main(): # Another process created it first, clean up temp if os.path.exists(temp_path): os.remove(temp_path) - print(f"User config was created by another process") - + print("User config was created by another process") + # Ensure node_id exists and matches hardware (add/update if needed, Pi only) ensure_node_id(user_config_path) - + # Overlay user config if it exists and has content if os.path.exists(user_config_path): print(f"Loading user config from {user_config_path}") - with open(user_config_path, 'r') as f: + with open(user_config_path) as f: user = yaml.safe_load(f) - + if user: # Only merge if user.yml has actual content print("Applying user overrides...") merge(config, user) else: print("User config is empty, using defaults") - + # Overlay forced config (highest priority) if os.path.exists(forced_config): print(f"Loading forced config from {forced_config}") - with open(forced_config, 'r') as f: + with open(forced_config) as f: forced = yaml.safe_load(f) - + if forced: # Only merge if content exists print("Applying forced overrides...") merge(config, forced) - + # Migrate legacy field formats forward (e.g. scalar -> per-tuner gainReduction) migrate_gain_reduction(config) @@ -273,7 +274,7 @@ def main(): generate_retina_tracker_config(config, os.path.dirname(output_config_path)) print("Config merge completed successfully!") - + except Exception as e: print(f"ERROR during config merge: {e}") import traceback @@ -281,4 +282,4 @@ def main(): sys.exit(1) if __name__ == "__main__": - main() \ No newline at end of file + main() diff --git a/config-merger/test/test_merge_config.py b/config-merger/test/test_merge_config.py index 2ed3ff3..e331c71 100755 --- a/config-merger/test/test_merge_config.py +++ b/config-merger/test/test_merge_config.py @@ -5,13 +5,14 @@ Adapted from blah2-arm tests for the simplified 3-argument version. """ -import unittest -import tempfile -import shutil import os -import sys +import shutil +import tempfile +import unittest + import yaml + class TestConfigMerge(unittest.TestCase): def setUp(self): @@ -33,7 +34,7 @@ def write_yaml(self, path, data): def read_yaml(self, path): """Helper to read YAML file""" - with open(path, 'r') as f: + with open(path) as f: return yaml.safe_load(f) def run_merge(self): @@ -226,7 +227,7 @@ def test_tar1090_env_generated(self): self.assertTrue(os.path.exists(env_path), "tar1090.env should be generated") # Read and verify contents - with open(env_path, 'r') as f: + with open(env_path) as f: env_content = f.read() self.assertIn('RECEIVER_LAT=-34.9192', env_content) @@ -269,7 +270,7 @@ def test_tar1090_env_adsblol_disabled(self): self.run_merge() env_path = os.path.join(self.config_dir, 'tar1090.env') - with open(env_path, 'r') as f: + with open(env_path) as f: env_content = f.read() self.assertIn('ADSBLOL_ENABLED=false', env_content) @@ -302,7 +303,7 @@ def test_tar1090_env_user_override(self): self.run_merge() env_path = os.path.join(self.config_dir, 'tar1090.env') - with open(env_path, 'r') as f: + with open(env_path) as f: env_content = f.read() # User overrides should apply @@ -378,7 +379,7 @@ def test_actual_config_files(self): env_path = os.path.join(self.config_dir, 'tar1090.env') self.assertTrue(os.path.exists(env_path), "tar1090.env should be generated with actual config") - with open(env_path, 'r') as f: + with open(env_path) as f: env_content = f.read() # Check expected values from default.yml (San Francisco location) @@ -411,7 +412,7 @@ def test_tar1090_env_with_adsb_source(self): env_path = os.path.join(self.config_dir, 'tar1090.env') self.assertTrue(os.path.exists(env_path)) - with open(env_path, 'r') as f: + with open(env_path) as f: env_content = f.read() self.assertIn('READSB_NET_CONNECTOR=192.168.8.183,30005,beast_in', env_content) @@ -439,7 +440,7 @@ def test_tar1090_env_without_adsb_source(self): env_path = os.path.join(self.config_dir, 'tar1090.env') self.assertTrue(os.path.exists(env_path)) - with open(env_path, 'r') as f: + with open(env_path) as f: env_content = f.read() self.assertNotIn('READSB_NET_CONNECTOR', env_content) @@ -465,7 +466,7 @@ def test_tar1090_env_uses_location_rx(self): self.run_merge() env_path = os.path.join(self.config_dir, 'tar1090.env') - with open(env_path, 'r') as f: + with open(env_path) as f: env_content = f.read() # Should use location.rx values diff --git a/ruff.toml b/ruff.toml new file mode 100644 index 0000000..c82d4f7 --- /dev/null +++ b/ruff.toml @@ -0,0 +1,44 @@ +# Shared ruff standard for offworldlabs Python repos. +# Keep in sync across repos; see offworldlabs/ops for the canonical copy. + +line-length = 120 +target-version = "py311" + +[lint] +select = [ + "E", # pycodestyle errors + "W", # pycodestyle warnings + "F", # pyflakes + "I", # isort + "B", # flake8-bugbear + "UP", # pyupgrade + "S", # flake8-bandit (security) + "SIM", # flake8-simplify +] +ignore = [ + "E501", # line too long — handled by formatter + "E402", # module-level import not at top — env setup before imports is intentional + "S101", # assert in tests is fine + "S104", # binding to 0.0.0.0 is intentional (Docker) + "S105", # hardcoded password false positives on dev defaults + "S106", # hardcoded password false positives + "S110", # try-except-pass is used intentionally + "S112", # try-except-continue is intentional in iteration + "S310", # URL open audit — URLs are constructed internally + "S311", # pseudo-random is fine for non-crypto uses + "S501", # requests without verify — internal calls + "S603", # subprocess calls are in controlled scripts + "S607", # partial executable path is fine for scripts + "B008", # function call in default arg — Depends() is FastAPI pattern + "B905", # zip strict — not needed everywhere + "SIM102", # nested if — readability preference + "SIM105", # contextlib.suppress — try/except is more explicit + "SIM108", # ternary operator — readability preference + "SIM117", # combine with statements — readability preference + "UP017", # datetime.UTC — cosmetic, timezone.utc is fine + "UP028", # yield from — explicit loop is clearer +] + +[lint.per-file-ignores] +"tests/*" = ["S", "B"] +"scripts/*" = ["S", "E"]