Skip to content

OSW-1617: Fix table relationships in CDB schema#104

Open
bbrondel wants to merge 6 commits intomainfrom
tickets/OSW-1617
Open

OSW-1617: Fix table relationships in CDB schema#104
bbrondel wants to merge 6 commits intomainfrom
tickets/OSW-1617

Conversation

@bbrondel
Copy link
Collaborator

@bbrondel bbrondel commented Jan 9, 2026

These migrations correspond to the schema changes in sdm_schemas in the tickets/OSW-1617 branch. They are designed to unify all of the instruments with a common schema.

This PR also includes some test scripts and other scripting improvements. The script validate_schema_migrations.py runs a few checks for a new migration, verifying that the completed migration matches the YAML file that generated it and that the downgrade feature correctly reverses the effect of the upgrade.

A review of these migration scripts on a locally generated database show that the migrations apply correctly and work as expected. The resulting schemas are the same across all instruments.

@bbrondel bbrondel force-pushed the tickets/OSW-1617 branch 6 times, most recently from 11d41a4 to 4255b35 Compare January 10, 2026 00:00

def _load_metadata(yaml_path: Path) -> sa.MetaData:
"""Build SQLAlchemy metadata from a Felis YAML schema."""
yaml_data = yaml.safe_load(yaml_path.read_text())
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I would recommend to use instead Schema.from_uri(path, context={"id_generation": True}) here and any other places where the schema is loaded into Felis.

b_path = args.schema_b.expanduser()
if not a_path.is_file() or not b_path.is_file():
print(f"Both inputs must be files. Got: {a_path} and {b_path}")
return 1
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Best to make this a variable which indicates what the return value is supposed to mean.

print("Differences found:")
for line in diffs:
print(f"- {line}")
return 2
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Also make this a variable.

@bbrondel bbrondel force-pushed the tickets/OSW-1617 branch 2 times, most recently from 7e690b3 to cd57b20 Compare January 19, 2026 22:06
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants