Upsert Tables For Configuration Changes#3
Conversation
There was a problem hiding this comment.
Pull request overview
This PR updates MetaTracker’s database initialization to support dynamic, SWxSOC-driven mission/instrument configuration and to make table population idempotent via upserts, including automatic schema extension when new instruments introduce new instrument_N_id columns.
Changes:
- Load mission/instrument configuration dynamically from
swxsocand generate instrument configurations programmatically. - Refactor table setup to be idempotent:
create_allonce, then upsert lookup/config tables; addsync_instrument_configuration_schema()for missing columns. - Add/adjust tests and CI tooling to validate idempotency, schema sync, and configuration-driven behavior.
Reviewed changes
Copilot reviewed 11 out of 21 changed files in this pull request and generated 9 comments.
Show a summary per file
| File | Description |
|---|---|
| metatracker/config/config.py | Replaces hardcoded defaults with SWxSOC-driven mission/instrument/config generation and adds CSV/JSON file types. |
| metatracker/database/tables/init.py | Implements upsert behavior for populate functions, refactors create_tables(), and adds schema sync via ALTER TABLE. |
| metatracker/database/tables/instrument_configuration_table.py | Minor formatting in dynamic column creation. |
| metatracker/database/tables/status_table.py | Import/order cleanup. |
| metatracker/tracker/tracker.py | Minor formatting/import normalization; small change to origin_file_ids access. |
| metatracker/init.py | Switches logging to swxsoc.log, adds config accessors, and exposes _test_files_directory for tests. |
| metatracker/tests/conftest.py | Sets SWXSOC_MISSION for tests and forces SWxSOC reconfiguration before collection. |
| metatracker/tests/test_upsert.py | New test suite covering idempotent upserts, updates, inserts, schema sync, and FK preservation. |
| metatracker/tests/test_tables.py | New tests for table helpers and table creation/removal. |
| metatracker/tests/test_database.py | New basic tests for engine/session/connection helpers. |
| metatracker/tests/test_cdftracker.py | New tests for logging and set_config/get_config. |
| metatracker/tests/test_tracker.py | Updates tests to use packaged test files; adjusts expected counts/behavior. |
| metatracker/tests/test_files/* | Adds test fixture files referenced by tests. |
| Makefile | Switches from poetry lock --check to poetry check in make check. |
| .github/workflows/main.yml | CI refresh: action version bumps, consolidate checks under make check, set SWXSOC_MISSION for tox. |
| .github/actions/setup-poetry-env/action.yml | Updates to newer setup-python/cache actions and defaults Python to 3.10. |
Comments suppressed due to low confidence (6)
metatracker/tests/test_tracker.py:17
TEST_SCIENCE_FILENAMEis assigned twice to the same value. Remove the duplicate assignment to avoid confusion and accidental divergence later.
metatracker/tests/test_tracker.py:156- Debug-only import/print statements in tests (
import swxsoc+ printing the mission name) will add noise to CI output and can mask failures. Please remove these or replace with assertions if you need to validate configuration.
metatracker/tests/test_tracker.py:357 assert len(instrument_configurations) == 7will break if the SWxSOC mission instruments change. Consider deriving the expected count from configuration (e.g.,len(CONFIGURATION.instrument_configurations)or2**len(CONFIGURATION.instruments) - 1).
metatracker/tests/test_tracker.py:407assert len(instrument_map) == 3is mission-dependent now that instrument definitions are loaded dynamically. Prefer asserting againstlen(CONFIGURATION.instruments)(or validating specific expected keys/values).
metatracker/tests/test_tracker.py:335assert len(instruments) == 3is brittle now that instruments come from SWxSOC mission config. Prefer asserting againstlen(CONFIGURATION.instruments)(or compare the actual instrument list toCONFIGURATION.instruments).
metatracker/tests/test_tracker.py:334print(instruments)in a unit test adds unnecessary noise to test output. Remove the print (or assert on the specific expected contents instead).
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Welcome to Codecov 🎉Once you merge this PR into your default branch, you're all set! Codecov will compare coverage reports and display results in all future pull requests. Thanks for integrating Codecov - We've got you covered ☂️ |
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 24 out of 31 changed files in this pull request and generated 9 comments.
Comments suppressed due to low confidence (4)
metatracker/tests/test_tracker.py:335
print(instruments)in a unit test will pollute CI output. Please remove the print statement and assert on the expected contents instead.
metatracker/tests/test_tracker.py:404map_instrument_list()expects an iterable of instrument IDs, but this test passes the full{id: short_name}dict returned byget_instruments()(and suppresses the type error). While iterating a dict happens to yield its keys, it’s implicit and brittle—prefer passinglist(instruments.keys())so the intent is explicit and thetype: ignorecan be removed.
metatracker/tests/test_tracker.py:18TEST_SCIENCE_FILENAMEis assigned twice to the same value. This looks accidental and makes the test constants harder to maintain—please remove the duplicate assignment.
metatracker/tests/test_tracker.py:156- There are debugging artifacts in the test (
import swxsocandprint(swxsoc.config[...])). Printing during test runs adds noise and can hide failures in CI logs. Please remove theprint(and the extra import if it's only for printing).
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
OK I know this is an enormous PR. Most of the changes are not related to the new functionality. There are a lot of formatting changes and changes for static type hinting that were needed for automated tests to pass. I don't have the logs from the last time Damian was making changes here, but it looks like these kinds of tests were still failing back then.
The files that contain functionality changes are:
metatracker/config/config.pymetatracker/database/tables/__init__.pyAdded
swxsocviaMetaTrackerConfiguration.from_swxsoc()— mission name, instruments, and instrument configurations are now derived from theSWXSOC_MISSIONenvironment variable at runtime instead of being hardcoded.populate_*functions — existing rows are updated in place rather than skipped, and new rows are inserted. Orphaned rows are preserved for foreign-key integrity.sync_instrument_configuration_schema()to automaticallyALTER TABLEand add missinginstrument_N_idcolumns when new instruments are introduced.test_upsert.pytest suite covering idempotency, insert, update, schema sync, and FK preservation.Changed
create_tables()refactored to callBase.metadata.create_allonce followed by upserts, removing the per-tableis_table_emptycheck pattern.closes #2