Code review fixes

core/data_store.py

@@ -106,6 +106,7 @@ class DataStore:
         and testing the callsign every time is expensive. So instead we build a separate in-memory lookup of compiled
         regex against DXCC entity code, as a list of tuples we can iterate through."""
 
+        self.dxcc_lookup_by_call_regex = []
         for entry in [DATA_STORE.dxcc_data[key] for key in DATA_STORE.dxcc_data]:
             self.dxcc_lookup_by_call_regex.append((re.compile(entry["prefixRegex"]), entry["entityCode"]))
 

core/live_data_cache.py

@@ -22,6 +22,7 @@ class LiveDataCache:
         self._listeners_lock = threading.Lock()
         self._snapshot_dir = snapshot_dir
         self._disk_cache = diskcache.Cache(str(snapshot_dir))
+        self._stop_event = threading.Event()
         self._load_snapshot()
         self._start_periodic_snapshot(snapshot_interval_sec)
 
@@ -89,13 +90,13 @@ class LiveDataCache:
 
     def _start_periodic_snapshot(self, interval):
         def loop():
-            while True:
-                time.sleep(interval)
+            while not self._stop_event.wait(timeout=interval):
                 self.save_snapshot()
 
-        t = threading.Thread(target=loop, name=f"LiveDataCache-Snapshot-{self._snapshot_dir}")
+        t = threading.Thread(target=loop, name=f"LiveDataCache-Snapshot-{self._snapshot_dir}", daemon=True)
         t.start()
 
     def close(self):
+        self._stop_event.set()
         self.save_snapshot()
         self._disk_cache.close()

core/sig_lookup_helper.py

@@ -24,7 +24,7 @@ def get_sig_ref_info(sig_name, ref_id):
 
     # Sometimes we allow spaces instead of dashes in references due to common usage that way, but official reference
     # lists never do, so convert them here.
-    ref_id.replace(" ", "-")
+    ref_id = ref_id.replace(" ", "-")
 
     # Prepare the object to be returned
     sig_ref = SIGRef(sig=sig_name, id=ref_id)
@@ -117,9 +117,9 @@ def get_sig_ref_info(sig_name, ref_id):
         try:
             lookup_data = DATA_STORE.sigrefs.get(key) if key in DATA_STORE.sigrefs else None
             if lookup_data:
-                for key, value in lookup_data.__dict__.items():
-                    if value is not None and sig_ref.__dict__.get(key) is None:
-                        sig_ref.__dict__[key] = value
+                for attr, value in lookup_data.__dict__.items():
+                    if value is not None and sig_ref.__dict__.get(attr) is None:
+                        sig_ref.__dict__[attr] = value
 
             else:
                 # Maybe a super new reference we don't know about yet, but more likely a typo or a test reference,

core/url_data_cache.py

@@ -14,14 +14,13 @@ class URLDataCache(CachedSession):
     used across multiple threads, though note that URL lookups will block each other this way, so it is still better to
     create one of these objects per thread if possible."""
 
-    _lock = threading.Lock()
-
     def __init__(self, name):
         super().__init__(
             f"{CACHE_DIR}urls/{name}",
             expire_after=timedelta(days=1),
             allowable_codes=(200, 400, 401, 403, 404),
         )
+        self._lock = threading.Lock()
 
     def get(self, *args, **kwargs):
         with self._lock:

data/alert.py

@@ -158,7 +158,7 @@ class Alert:
                 self.icon = "fa-globe-africa"
             elif self.alert_type == AlertType.CONTEST:
                 self.icon = "fa-trophy"
-            elif self.alert_type == AlertType.CONTEST:
+            elif self.alert_type == AlertType.SATELLITE:
                 self.icon = "fa-satellite"
             elif self.sig_refs and self.sig_refs[0].icon:
                 self.icon = self.sig_refs[0].icon

data/spot.py

@@ -12,6 +12,7 @@ from pyhamtools.locator import latlong_to_locator, locator_to_latlong
 from core.call_lookup_helper import get_call_info
 from core.config import MAX_SPOT_AGE
 from core.constants import PROPAGATION_MODES, SIGS
+from core.data_store import DATA_STORE
 from core.enums import Continent, LocationSourceForSpot, Mode, ModeSource, ModeType
 from core.geo_utils import lat_lon_to_cq_zone, lat_lon_to_itu_zone
 from core.sig_lookup_helper import populate_missing_sig_ref_info
@@ -363,11 +364,13 @@ class Spot:
             if self.propagation_mode == "Satellite":
                 if not self.sig:
                     self.sig = "AMSAT"
-                self.sig_refs.append(SIGRef(sig="AMSAT"))
+                if not any(sig_ref.sig == "AMSAT" for sig_ref in self.sig_refs):
+                    self.sig_refs.append(SIGRef(sig="AMSAT"))
             if self.propagation_mode == "Earth-Moon-Earth":
                 if not self.sig:
                     self.sig = "EME"
-                self.sig_refs.append(SIGRef(sig="EME"))
+                if not any(sig_ref.sig == "EME" for sig_ref in self.sig_refs):
+                    self.sig_refs.append(SIGRef(sig="EME"))
 
             # Parse "de_grid -> dx_grid" structures from the comment
             if self.comment:
@@ -445,6 +448,15 @@ class Spot:
                 elif self.dx_call:
                     self.dx_itu_zone = dx_call_info.itu_zone
 
+            # DXCC lookup from callsign if nothing else has provided it
+            if self.dx_call and not self.dx_dxcc_id:
+                for regex, entity_code in DATA_STORE.dxcc_lookup_by_call_regex:
+                    if regex.pattern and regex.match(self.dx_call):
+                        self.dx_dxcc_id = entity_code
+                        break
+            if self.dx_dxcc_id and not self.dx_flag:
+                self.dx_flag = get_flag_for_dxcc(self.dx_dxcc_id)
+
             # DX Location is "good" if it is from a spot, or from QRZ if the callsign doesn't contain a slash, so the operator
             # is likely at home.
             self.dx_location_good = bool(

providers/spot/aprsis.py

@@ -1,6 +1,6 @@
 import logging
 from datetime import datetime
-from threading import Thread
+from threading import Event, Thread
 
 import aprslib
 import pytz
@@ -17,49 +17,68 @@ class APRSIS(SpotProvider):
 
     def __init__(self, provider_config):
         super().__init__("APRS-IS", provider_config)
-        self._thread = Thread(target=self._connect, name="APRSISSpotProvider")
+        self._thread = Thread(target=self._run, name="APRSISSpotProvider")
         self._thread.daemon = True
         self._aprsis = None
+        self._running = True
+        self._stop_event = Event()
 
     def start(self):
         self._thread.start()
 
-    def _connect(self):
-        self._aprsis = aprslib.IS(SERVER_OWNER_CALLSIGN)
-        self.status = "Connecting"
-        logger.info("APRS-IS connecting...")
-        self._aprsis.connect()
-        self._aprsis.consumer(self._handle)
-        logger.info("APRS-IS connected.")
+    def _run(self):
+        while self._running:
+            try:
+                self._aprsis = aprslib.IS(SERVER_OWNER_CALLSIGN)
+                self.status = "Connecting"
+                logger.info("APRS-IS connecting...")
+                self._aprsis.connect()
+                logger.info("APRS-IS connected.")
+                self._aprsis.consumer(self._handle, immortal=True)
+
+            except Exception:
+                if self._running:
+                    self.status = "Error"
+                    logger.exception("Exception in APRS-IS provider")
+
+            if self._running:
+                self._stop_event.wait(timeout=5)
 
     def stop(self):
+        self._running = False
         self.status = "Shutting down"
-        self._aprsis.close()
+        self._stop_event.set()
+        if self._aprsis:
+            self._aprsis.close()
         self._thread.join()
 
     def _handle(self, data):
-        # Split SSID in "from" call and store separately
-        from_parts = str(data["from"]).split("-")
-        dx_call = from_parts[0].upper()
-        dx_ssid = from_parts[1].upper() if len(from_parts) > 1 else None
-        via_parts = str(data["via"]).split("-")
-        de_call = via_parts[0].upper()
-        de_ssid = via_parts[1].upper() if len(via_parts) > 1 else None
-        spot = Spot(
-            source="APRS-IS",
-            dx_call=dx_call,
-            dx_ssid=dx_ssid,
-            de_call=de_call,
-            de_ssid=de_ssid,
-            comment=str(data["comment"]) if "comment" in data else None,
-            dx_latitude=float(data["latitude"]) if "latitude" in data else None,
-            dx_longitude=float(data["longitude"]) if "longitude" in data else None,
-            time=datetime.now(pytz.UTC).timestamp(),
-        )  # APRS-IS spots are live so we can assume spot time is "now"
+        try:
+            # Split SSID in "from" call and store separately
+            from_parts = str(data["from"]).split("-")
+            dx_call = from_parts[0].upper()
+            dx_ssid = from_parts[1].upper() if len(from_parts) > 1 else None
+            via_parts = str(data["via"]).split("-")
+            de_call = via_parts[0].upper()
+            de_ssid = via_parts[1].upper() if len(via_parts) > 1 else None
+            spot = Spot(
+                source="APRS-IS",
+                dx_call=dx_call,
+                dx_ssid=dx_ssid,
+                de_call=de_call,
+                de_ssid=de_ssid,
+                comment=str(data["comment"]) if "comment" in data else None,
+                dx_latitude=float(data["latitude"]) if "latitude" in data else None,
+                dx_longitude=float(data["longitude"]) if "longitude" in data else None,
+                time=datetime.now(pytz.UTC).timestamp(),
+            )  # APRS-IS spots are live so we can assume spot time is "now"
 
-        # Add to our list
-        self._submit(spot)
+            # Add to our list
+            self._submit(spot)
 
-        self.status = "OK"
-        self.last_update_time = datetime.now(pytz.UTC)
-        logger.debug("Data received from APRS-IS.")
+            self.status = "OK"
+            self.last_update_time = datetime.now(pytz.UTC)
+            logger.debug("Data received from APRS-IS.")
+
+        except Exception:
+            logger.exception("Exception handling APRS-IS packet")

providers/spot/websocket_spot_provider.py

@@ -50,8 +50,13 @@ class WebsocketSpotProvider(SpotProvider):
                 self.status = "Connecting"
                 self._ws = create_connection(self._url, header=HTTP_HEADERS)
                 self.status = "Connected"
-                data = self._ws.recv()
-                if data:
+
+                # Keep reading from this same connection until it drops or we're asked to stop, rather than
+                # reconnecting for every message.
+                while not self._stopped:
+                    data = self._ws.recv()
+                    if not data:
+                        break
                     try:
                         new_spot = self._ws_message_to_spot(data)
                         if new_spot:
@@ -69,7 +74,16 @@ class WebsocketSpotProvider(SpotProvider):
                 logger.exception(f"Exception in Websocket Spot Provider ({self.name})")
             else:
                 self.status = "Disconnected"
-            sleep(5)  # Wait before trying to reconnect
+            finally:
+                if self._ws:
+                    try:
+                        self._ws.close()
+                    except Exception:
+                        # No problem, we were getting rid of this object anyway.
+                        pass
+                    self._ws = None
+            if not self._stopped:
+                sleep(5)  # Wait before trying to reconnect
 
     def _ws_message_to_spot(self, b):
         """Convert a WS message received from the API into a spot. The exact message data (in bytes) is provided here so the

telnetserver/telnetserver.py

@@ -163,9 +163,10 @@ class TelnetServer:
         # Ensure ASCII formatting for telnet clients
         encoded_line = self._format_dxspider_spot(spot).encode("ascii", errors="ignore")
 
-        # Try to write to all clients, and in the process find the ones that are disconnected
+        # Try to write to all clients, and in the process find the ones that are disconnected. Iterate over a copy
+        # since a new client can connect (mutating self._clients) while we're awaiting a write below.
         disconnected_clients = set()
-        for writer in self._clients:
+        for writer in list(self._clients):
             try:
                 writer.write(encoded_line)
                 await writer.drain()

templates/add_spot.html

@@ -77,7 +77,7 @@
 
 </div>
 
-<script src="/static/js/add-spot.js?v=1789713510"></script>
+<script src="/static/js/add-spot.js?v=1789714568"></script>
 <script>$(document).ready(function () {
     $("#nav-link-add-spot").addClass("active");
 }); <!-- highlight active page in nav --></script>

templates/alerts.html

@@ -83,7 +83,7 @@
 
 </div>
 
-<script src="/static/js/alerts.js?v=1789713511"></script>
+<script src="/static/js/alerts.js?v=1789714568"></script>
 <script>$(document).ready(function () {
     $("#nav-link-alerts").addClass("active");
 }); <!-- highlight active page in nav --></script>

templates/bands.html

@@ -76,8 +76,8 @@
 
 </div>
 
-<script src="/static/js/spotsbandsandmap.js?v=1789713510"></script>
-<script src="/static/js/bands.js?v=1789713510"></script>
+<script src="/static/js/spotsbandsandmap.js?v=1789714568"></script>
+<script src="/static/js/bands.js?v=1789714568"></script>
 <script>$(document).ready(function () {
     $("#nav-link-bands").addClass("active");
 }); <!-- highlight active page in nav --></script>

templates/base.html

@@ -1,6 +1,6 @@
 {% extends "skeleton.html" %}
 {% block head_extra %}
-<link rel="stylesheet" href="/static/css/style.css?v=1789713510" type="text/css">
+<link rel="stylesheet" href="/static/css/style.css?v=1789714568" type="text/css">
 <link href="/static/vendor/css/bootstrap-5.3.8.min.css" rel="stylesheet">
 <link href="/static/vendor/css/fontawesome-6.7.2.min.css" rel="stylesheet">
 <link href="/static/vendor/css/solid-6.7.2.min.css" rel="stylesheet">
@@ -16,10 +16,10 @@
     window.fetchEventSource = fetchEventSource;
 </script>
 
-<script src="/static/js/utils.js?v=1789713510"></script>
-<script src="/static/js/ui-ham.js?v=1789713510"></script>
-<script src="/static/js/geo.js?v=1789713510"></script>
-<script src="/static/js/common.js?v=1789713510"></script>
+<script src="/static/js/utils.js?v=1789714568"></script>
+<script src="/static/js/ui-ham.js?v=1789714568"></script>
+<script src="/static/js/geo.js?v=1789714568"></script>
+<script src="/static/js/common.js?v=1789714568"></script>
 {% end %}
 {% block body %}
 <div class="container">

templates/conditions.html

@@ -284,7 +284,7 @@
 </div>
 
 <script src="/static/vendor/js/chart-4.4.9.umd.min.js"></script>
-<script src="/static/js/conditions.js?v=1789713510"></script>
+<script src="/static/js/conditions.js?v=1789714568"></script>
 <script>$(document).ready(function () {
     $("#nav-link-conditions").addClass("active");
 }); <!-- highlight active page in nav --></script>

templates/map.html

@@ -113,8 +113,8 @@
     const CARTODB_API_KEY = "{{ web_ui_options.get('cartodb_api_key', '') }}";
 </script>
 
-<script src="/static/js/spotsbandsandmap.js?v=1789713511"></script>
-<script src="/static/js/map.js?v=1789713511"></script>
+<script src="/static/js/spotsbandsandmap.js?v=1789714568"></script>
+<script src="/static/js/map.js?v=1789714568"></script>
 <script>$(document).ready(function () {
     $("#nav-link-map").addClass("active");
 }); <!-- highlight active page in nav --></script>

templates/spots.html

@@ -125,8 +125,8 @@
 
 </div>
 
-<script src="/static/js/spotsbandsandmap.js?v=1789713510"></script>
-<script src="/static/js/spots.js?v=1789713510"></script>
+<script src="/static/js/spotsbandsandmap.js?v=1789714568"></script>
+<script src="/static/js/spots.js?v=1789714568"></script>
 <script>$(document).ready(function () {
     $("#nav-link-spots").addClass("active");
 }); <!-- highlight active page in nav --></script>

templates/status.html

@@ -96,7 +96,7 @@
     </div>
 </div>
 
-<script src="/static/js/status.js?v=1789713510"></script>
+<script src="/static/js/status.js?v=1789714568"></script>
 <script>
     $(document).ready(function () {
         $("#nav-link-status").addClass("active");

webserver/handlers/api/v1_compatability.py

@@ -34,7 +34,7 @@ class V1RedirectHandler(tornado.web.RequestHandler):
             response = await client.fetch(
                 new_url,
                 method=self.request.method,
-                headers=self.request.headers,
+                headers=headers,
                 body=None if self.request.method == "GET" else (self.request.body or b""),
                 raise_error=False,
                 follow_redirects=False,