Space-Track to PostgreSQL Satellite Synchronization Pipeline - #149
Conversation
|
Skipping CodeAnt AI review — this PR is a back-merge between long-lived branches ( If you want to analyze this anyway (e.g. you resolved conflicts with new logic), comment |
github-actions
Bot
added
backend
📝 WalkthroughWalkthroughSpace-Track synchronization now adds targeted authentication and fetch error handling, validates API payloads, uses timezone-aware document timestamps, improves PostgreSQL upserts, and reports inserted, updated, failed, and skipped records across sync operations. ChangesSpace-Track synchronization
Estimated code review effort: 3 (Moderate) | ~20 minutes Sequence Diagram(s)sequenceDiagram
participant sync_group
participant SpaceTrackAPI
participant _bulk_upsert
participant PostgreSQL
sync_group->>SpaceTrackAPI: authenticate and fetch group records
SpaceTrackAPI-->>sync_group: return validated records
sync_group->>_bulk_upsert: transform and persist documents
_bulk_upsert->>PostgreSQL: insert or update satellite rows
PostgreSQL-->>_bulk_upsert: return persistence counts
_bulk_upsert-->>sync_group: return inserted, updated, failed, and skipped counts
Possibly related PRs
Suggested reviewers: 🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![greptile-apps[bot]](https://avatars.githubusercontent.com/in/867647?s=60&v=4){kind=small}
| ) | ||
| db.execute(stmt) | ||
| result = db.execute(stmt) | ||
| db.commit() |
There was a problem hiding this comment.
When PostgreSQL processes records whose noradId already exists, ON CONFLICT DO UPDATE includes those updated rows in result.rowcount, but the code assigns the entire count to inserted and leaves updated at zero. This makes API responses, aggregate statistics, and sync_by_catalog logs report refreshed records as new inserts.
Prompt To Fix With AI
This is a comment left during a code review.
Path: backend/orbital/spacetrack.py
Line: 295
Comment:
**Conflict updates counted as inserts**
When PostgreSQL processes records whose `noradId` already exists, `ON CONFLICT DO UPDATE` includes those updated rows in `result.rowcount`, but the code assigns the entire count to `inserted` and leaves `updated` at zero. This makes API responses, aggregate statistics, and `sync_by_catalog` logs report refreshed records as new inserts.
---
For each issue above, determine whether it is valid and should be fixed. If so, fix it directly.There was a problem hiding this comment.
Pull request overview
Fixes the Space-Track ingestion/sync pipeline so fetched satellite records are persisted via bulk upsert, and adds richer operational logging and per-group sync metrics to help diagnose ingestion failures and frontend “0 satellites” symptoms.
Changes:
- Improves Space-Track auth/fetch resiliency and logging, including clearer provider-chain failure reporting.
- Refactors ingestion persistence into a bulk UPSERT path (PostgreSQL) with a row-by-row fallback for SQLite/tests, and expands sync status output (inserted/updated).
- Removes a redundant
index=TruefromSpaceWeather.recorded_atwhile retaining the explicitIndex(...)definition.
Reviewed changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated 3 comments.
| File | Description |
|---|---|
| backend/orbital/spacetrack.py | Adds logging, improves error handling, and refactors persistence into a bulk UPSERT with expanded sync result metrics. |
| backend/models/db_models.py | Removes redundant column-level indexing for SpaceWeather.recorded_at (explicit table index remains). |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| result = db.execute(stmt) | ||
| db.commit() | ||
| written = len(docs) | ||
| inserted = result.rowcount |
| Returns (inserted, updated, failed_ids, skipped_ids). | ||
| Skipped ids are records that already exist and were updated in place. |
| now = datetime.datetime.now(datetime.timezone.utc) | ||
|
|
||
| if not norad_id: | ||
| logger.warning(f"[SpaceTrack] Record missing NORAD_CAT_ID: {rec.get('OBJECT_NAME', '?')}") |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 3
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@backend/orbital/spacetrack.py`:
- Around line 154-161: Update the shared _send_request() failure path to call
_reset_auth() when the Space-Track request raises a 401 or 403 authorization
error, before re-raising the exception. Ensure this behavior applies to callers
such as fetch_group_json() and fetch_by_catalog_json(), while leaving
non-authorization failures unchanged.
- Around line 260-267: The PostgreSQL upsert flow around the method returning
inserted, updated, failed_ids, and skipped_ids must distinguish newly inserted
rows from conflict updates instead of relying on result.rowcount. Return or
derive a per-row conflict outcome, such as a RETURNING flag identifying existing
rows, accumulate the corresponding skipped_ids, and propagate the separate
metric through sync_group and sync_all_groups while preserving the
non-PostgreSQL fallback.
- Around line 302-317: Update the batch commit flow in _bulk_upsert so commit
failures are caught, db.rollback() is called to reset the session, and all rows
in the failed batch are marked as failed while inserted and updated counters are
cleared. Report the commit error consistently with the existing row-level
warning handling before allowing later groups to reuse the session.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro Plus
Run ID: b586a60f-e60e-4e23-b746-04e2964683aa
📒 Files selected for processing (2)
backend/models/db_models.pybackend/orbital/spacetrack.py
| logger.debug(f"[SpaceTrack] GET {url}") | ||
| resp = self.client.get(url) | ||
| resp.raise_for_status() | ||
| data = resp.json() | ||
| if not isinstance(data, list): | ||
| self._reset_auth() | ||
| logger.warning(f"[SpaceTrack] Unexpected response type {type(data)} — resetting auth") | ||
| raise ValueError(f"Unexpected Space-Track response type: {type(data)}") |
There was a problem hiding this comment.
🩺 Stability & Availability | 🟠 Major | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "== files =="
git ls-files | rg 'orbital/spacetrack|sync|catalog|group' || true
echo "== spacetrack outline =="
ast-grep outline backend/orbital/spacetrack.py || true
echo "== relevant spacetrack sections =="
wc -l backend/orbital/spacetrack.py
sed -n '1,260p' backend/orbital/spacetrack.pyRepository: 7-Blocks/Kepler
Length of output: 11812
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "== read rest of spacetrack relevant methods =="
sed -n '228,476p' backend/orbital/spacetrack.py
echo "== call sites of fetch methods =="
rg -n "fetch_group_json|fetch_by_catalog_json|sync_group|sync_all_groups|sync_by_catalog|space-track|space_track|SpaceTrack" backend -S
echo "== static verifier for _authenticated lifecycle around _send_request =="
python3 - <<'PY'
from pathlib import Path
p = Path('backend/orbital/spacetrack.py')
s = p.read_text()
checks = {
"authenticated_flag_exists": "self._authenticated" in s,
"authenticate_sets_authenticated_on_200_cookie": '"spacetrack_session" in self.client.cookies' in s and "self._authenticated = True" in s,
"reset_only_copies_flag": s.count("def _reset_auth(self):") == 1 and "self._authenticated = False" in s,
"request_only_calls_raise_for_status": (
"def _send_request(self, url: str) -> List[Dict[str, Any]]:" in s
and "resp.raise_for_status()" in s
),
"sender_decorated_retries_HTTPStatusError": "_retry(HTTP_MAX_ATTEMPTS, HTTP_BASE_DELAY, HTTP_MAX_DELAY," in s and "retry_on=(httpx.RequestError, httpx.HTTPStatusError)" in s,
}
for k, v in checks.items():
print(f"{k}: {v}")
# Check if reset_auth occurs inside request success path before response.json and not tied to unexpected JSON
idx_request = s.index("def _send_request(self, url: str) -> List[Dict[str, Any]]:")
idx_end = s.index("def fetch_group_json(self, group: str", idx_request)
body = s[idx_request:idx_end]
print("reset_in_send_request_body:", "def _reset_auth(self):" in body or ".reset_auth()" in body)
PYRepository: 7-Blocks/Kepler
Length of output: 23388
Reset cached Space-Track auth on repeated authorization failures.
_authenticated stays True after a 401/403, so later calls skip a fresh login and reuse the bad session through fetch_group_json() and fetch_by_catalog_json(). Reset auth in the shared _send_request() failure path before re-raising.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@backend/orbital/spacetrack.py` around lines 154 - 161, Update the shared
_send_request() failure path to call _reset_auth() when the Space-Track request
raises a 401 or 403 authorization error, before re-raising the exception. Ensure
this behavior applies to callers such as fetch_group_json() and
fetch_by_catalog_json(), while leaving non-authorization failures unchanged.
| ) -> Tuple[int, int, List[str], List[str]]: | ||
| """ | ||
| Upsert using PostgreSQL INSERT ... ON CONFLICT (noradId) DO UPDATE SET ... | ||
| Falls back to individual merge for non-PostgreSQL dialects (e.g. SQLite in tests). | ||
|
|
||
| Returns (inserted, updated, failed_ids, skipped_ids). | ||
| Skipped ids are records that already exist and were updated in place. | ||
| """ |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟠 Major | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "Files matching spacetrack.py:"
fd -a 'spacetrack.py' . || true
echo
echo "Context around target lines:"
if [ -f backend/orbital/spacetrack.py ]; then
sed -n '230,450p' backend/orbital/spacetrack.py | cat -n | sed 's/^/backend\/orbital\/spacetrack.py:/'
fi
echo
echo "Search for upsert/sync/skipped/rowcount mentions:"
rg -n "upsert|INSERT.*ON CONFLICT|result\.rowcount|sync_group|skipped|updated_ids|inserted_ids|failed_ids" backend -S || trueRepository: 7-Blocks/Kepler
Length of output: 22306
🌐 Web query:
SQLAlchemy pg_insert result.rowcount PostgreSQL ON CONFLICT DO UPDATE affected rows inserts plus updates
💡 Result:
When using PostgreSQL's ON CONFLICT DO UPDATE (upsert) in SQLAlchemy, the rowcount returned by the execution result represents the total number of rows that were processed by the statement—specifically, those that were either inserted or updated [1][2]. Crucially, this rowcount does not reliably distinguish between an insert and an update, nor does it necessarily indicate that data was changed [1][3]. If an ON CONFLICT DO UPDATE statement targets an existing row and performs an update that sets values to their current state, PostgreSQL still counts that row as "affected" [1][3]. Consequently, the result.rowcount will return 1 for that row, even if no data values actually changed [1][3]. To accurately track or detect whether an actual change occurred, or to differentiate between an insert and an update, you should use the PostgreSQL RETURNING clause [1][4]. By appending.returning to your SQLAlchemy Insert object, you can retrieve the state of the rows after the operation [4][5]. This allows you to inspect the returned values—such as by comparing "old" and "new" values if you return both the affected row and the excluded data—to determine if the operation resulted in a meaningful modification [1][6]. Example pattern using RETURNING to detect changes: stmt = insert(table).values(...).on_conflict_do_update(...) # Add a clause to return both the current row and the excluded (proposed) values stmt = stmt.returning(table, literal_column("excluded.*")) result = conn.execute(stmt) In this approach, by comparing the returned columns, you can definitively verify which rows were modified versus simply re-inserted or updated to the same values [1].
Citations:
- 1: https://theneuralbase.com/postgresql-for-ai/learn/beginner/conflict-handling-patterns/
- 2: https://www.postgresql.org/docs/19/sql-insert.html
- 3: UPSERT rowCount is faulty brianc/node-postgres#1546
- 4: https://docs.sqlalchemy.org/en/21/orm/queryguide/dml.html
- 5: https://docs.sqlalchemy.org/en/20/core/dml.html
- 6: https://www.postgresql.org/docs/18/dml-returning.html
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "Inspect SQLAlchemy version if present and any SQL dialect checks:"
python3 - <<'PY'
try:
import sqlalchemy
from sqlalchemy.dialects.postgresql import insert as pg_insert
from sqlalchemy.testing import is
print("sqlalchemy_version", sqlalchemy.__version__)
except Exception as exc:
print("sqlalchemy_probe_error", type(exc).__name__, str(exc))
PY
echo
echo "Inspect return type annotation and returned skipped fields:"
rg -n "skipped|_bulk_upsert|sync_group|sync_all_groups|sync_by_catalog|upserted|inserted|updated" backend/orbital/spacetrack.py backend/api/v1/endpoints/catalog.py backend/tests/test_ingestion.py backend/app/tasks/celery_tasks.py
echo
echo "Read relevant returned status consumers:"
sed -n '170,215p' backend/api/v1/endpoints/catalog.py | cat -n
sed -n '15,30p' backend/app/tasks/celery_tasks.py | cat -n
sed -n '120,162p' backend/tests/test_ingestion.py | cat -nRepository: 7-Blocks/Kepler
Length of output: 347
Track PostgreSQL upsert inserts, updates, and skipped rows separately.
result.rowcount for PostgreSQL INSERT ... ON CONFLICT DO UPDATE is the sum of affected rows and cannot distinguish inserts from conflict updates. Classify each row by returning the conflict outcome (for example via a RETURNING flag that identifies existing rows), accumulate skipped_ids, and expose that metric from sync_group/sync_all_groups.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@backend/orbital/spacetrack.py` around lines 260 - 267, The PostgreSQL upsert
flow around the method returning inserted, updated, failed_ids, and skipped_ids
must distinguish newly inserted rows from conflict updates instead of relying on
result.rowcount. Return or derive a per-row conflict outcome, such as a
RETURNING flag identifying existing rows, accumulate the corresponding
skipped_ids, and propagate the separate metric through sync_group and
sync_all_groups while preserving the non-PostgreSQL fallback.
| for doc in docs: | ||
| try: | ||
| safe_doc = {k: v for k, v in doc.items() if k in model_cols} | ||
| existing = db.query(model).filter(model.noradId == doc["noradId"]).first() | ||
| if existing: | ||
| for k, v in doc.items(): | ||
| for k, v in safe_doc.items(): | ||
| if k not in ("id", "noradId", "createdAt") and hasattr(existing, k): | ||
| setattr(existing, k, v) | ||
| updated += 1 | ||
| else: | ||
| db.add(model(**doc)) | ||
| written += 1 | ||
| db.add(model(**safe_doc)) | ||
| inserted += 1 | ||
| except Exception as exc: | ||
| logger.warning(f"[SpaceTrack] Row upsert failed for {doc.get('noradId')}: {exc}") | ||
| failed.append(doc.get("noradId", "?")) | ||
| db.commit() |
There was a problem hiding this comment.
🩺 Stability & Availability | 🟡 Minor | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "== locate spacetrack.py =="
fd -a 'spacetrack\.py$' . || true
echo "== file outline around candidate =="
if [ -f backend/orbital/spacetrack.py ]; then
wc -l backend/orbital/spacetrack.py
ast-grep outline backend/orbital/spacetrack.py --view expanded | sed -n '1,220p' || true
echo "== lines 230-350 =="
sed -n '230,350p' backend/orbital/spacetrack.py | cat -n | sed 's/^/230+/'
fi
echo "== searches for sync_all_groups and model_cols update paths =="
rg -n "sync_all_groups|fallback|model_cols|SpaceTrack|db.commit|db.rollback|opt" backend || true
echo "== git diff stat/name =="
git diff --stat || true
git diff -- backend/orbital/spacetrack.py | sed -n '1,240p' || trueRepository: 7-Blocks/Kepler
Length of output: 20062
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "== sync_group/sync_all_groups sections =="
sed -n '339,425p' backend/orbital/spacetrack.py | cat -n | sed 's/^/339+/'
sed -n '408,475p' backend/orbital/spacetrack.py | cat -n | sed 's/^/408+/'
echo "== SQLAlchemy session rollback state behavior =="
python3 - <<'PY'
try:
from sqlalchemy import create_engine, text
from sqlalchemy.engine import Connection
from sqlalchemy.orm import Session
except Exception as exc:
print(f"SQLAlchemy unavailable: {exc}")
raise SystemExit(0)
engine = create_engine("sqlite:///:memory:")
with engine.connect() as conn:
conn.execute(text("CREATE TABLE foo (id INTEGER PRIMARY KEY, val TEXT)"))
conn.commit()
# Session that does no work; commit should not leave failed transaction.
session = Session(engine)
try:
session.commit()
print("empty_session_commit_ok=True")
except Exception as exc:
print(f"empty_session_commit_error={type(exc).__name__}: {exc}")
print(f"empty_session_failed={session.is_active}")
# Session that adds data, then flush raises.
session = Session(engine)
session.execute(text("INSERT INTO foo (id, val) VALUES (1, 'a')"))
# Force an error after the pending insert is in transaction (by failing commit on an explicit transaction).
try:
session.begin()
session.add(conn) # bad type; will fail while preparing/execution context is established
session.commit()
except Exception as exc:
print(f"bad_prepared_commit_error={type(exc).__name__}: {exc}")
print(f"bad_prepared_failed={session.is_active}")
session.rollback()
print(f"bad_prepared_after_rollback_active={session.is_active}")
# Check rollback vs rollback after already-failed state.
session = Session(engine)
session.begin()
try:
session.flush() # no insert; safe
raise ValueError("simulate commit flush failure")
except Exception:
print("pre-rollback_active=", session.is_active)
session.rollback()
print("post-rollback_active=", session.is_active)
# In SQLAlchemy, rollbacks are idempotent.
PY
echo "== test file around sync batch rollback assertions =="
sed -n '120,190p' backend/tests/test_ingestion.py | cat -n | sed 's/^/120+/' || trueRepository: 7-Blocks/Kepler
Length of output: 11633
Roll back the fallback upsert when commit fails.
The final db.commit() in backend/orbital/spacetrack.py:320 is outside the row-loop try, so if SQLAlchemy rolls the batch back during flush/commit the session stays in a failed transaction and later groups call _bulk_upsert() on that same session. Catch commit failures, call db.rollback(), and report/mark those rows as failed/fresh counters cleared.
Proposed fix
- db.commit()
+ try:
+ db.commit()
+ except Exception as exc:
+ db.rollback()
+ logger.error(f"[SpaceTrack] Fallback upsert commit failed: {exc}", exc_info=True)
+ failed = [d.get("noradId", "?") for d in docs]
+ inserted = updated = 0📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| for doc in docs: | |
| try: | |
| safe_doc = {k: v for k, v in doc.items() if k in model_cols} | |
| existing = db.query(model).filter(model.noradId == doc["noradId"]).first() | |
| if existing: | |
| for k, v in doc.items(): | |
| for k, v in safe_doc.items(): | |
| if k not in ("id", "noradId", "createdAt") and hasattr(existing, k): | |
| setattr(existing, k, v) | |
| updated += 1 | |
| else: | |
| db.add(model(**doc)) | |
| written += 1 | |
| db.add(model(**safe_doc)) | |
| inserted += 1 | |
| except Exception as exc: | |
| logger.warning(f"[SpaceTrack] Row upsert failed for {doc.get('noradId')}: {exc}") | |
| failed.append(doc.get("noradId", "?")) | |
| db.commit() | |
| for doc in docs: | |
| try: | |
| safe_doc = {k: v for k, v in doc.items() if k in model_cols} | |
| existing = db.query(model).filter(model.noradId == doc["noradId"]).first() | |
| if existing: | |
| for k, v in safe_doc.items(): | |
| if k not in ("id", "noradId", "createdAt") and hasattr(existing, k): | |
| setattr(existing, k, v) | |
| updated += 1 | |
| else: | |
| db.add(model(**safe_doc)) | |
| inserted += 1 | |
| except Exception as exc: | |
| logger.warning(f"[SpaceTrack] Row upsert failed for {doc.get('noradId')}: {exc}") | |
| failed.append(doc.get("noradId", "?")) | |
| try: | |
| db.commit() | |
| except Exception as exc: | |
| db.rollback() | |
| logger.error(f"[SpaceTrack] Fallback upsert commit failed: {exc}", exc_info=True) | |
| failed = [d.get("noradId", "?") for d in docs] | |
| inserted = updated = 0 |
🧰 Tools
🪛 Ruff (0.15.21)
[warning] 314-314: Do not catch blind exception: Exception
(BLE001)
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@backend/orbital/spacetrack.py` around lines 302 - 317, Update the batch
commit flow in _bulk_upsert so commit failures are caught, db.rollback() is
called to reset the session, and all rows in the failed batch are marked as
failed while inserted and updated counters are cleared. Report the commit error
consistently with the existing row-level warning handling before allowing later
groups to reuse the session.
|
Hi @TheLinuxGuy-ssh, your PR deployment has failed on Vercel. Could you please check the deployment logs and fix the issue? Thanks! 
|
Summary
Fix the satellite data ingestion pipeline where records fetched from Space-Track were not being persisted to PostgreSQL, causing the frontend to display 0 active satellites on the Cesium globe and dashboard.
Related Issue
Fixes #135
Type of Change
Screenshots / Screen Recordings
N/A
Testing Performed
Breaking Changes
No Breaking Changes
Checklist
ECSoC26 Submission
ECSoC26-L1– BeginnerECSoC26-L2– IntermediateECSoC26-L3– AdvancedSummary by CodeRabbit
Greptile Summary
This PR revises the Space-Track ingestion pipeline and its operational reporting.
SpaceWeather.recorded_at.Confidence Score: 4/5
The incorrect PostgreSQL insert/update accounting should be fixed before merging because synchronization APIs and logs currently misreport refreshed records as new inserts.
PostgreSQL uses a combined affected-row count for a statement that both inserts and conflict-updates records, while the changed response contract presents that count as inserts and always reports zero updates.
Files Needing Attention: backend/orbital/spacetrack.py
Important Files Changed
SpaceWeather.recorded_atwithout introducing a functional correctness issue in the reviewed change.Flowchart
%%{init: {'theme': 'neutral'}}%% flowchart LR A[Space-Track or fallback provider] --> B[Normalize GP records] B --> C[Filter model columns] C --> D{Database dialect} D -->|PostgreSQL| E[Bulk INSERT ON CONFLICT UPDATE] D -->|Other| F[Row-by-row merge] E --> G[Sync status counters] F --> G G --> H[API and task summaries]Prompt To Fix All With AI
Reviews (1): Last reviewed commit: "Space-Track to PostgreSQL Satellite Sync..." | Re-trigger Greptile