-
Notifications
You must be signed in to change notification settings - Fork 1
Network keys #250
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Merged
Merged
Network keys #250
Changes from all commits
Commits
Show all changes
10 commits
Select commit
Hold shift + click to select a range
e2e131f
Add migrations to add "network_key" column to allow network renames
Nospamas 8e1197f
black project
Nospamas 0a8378b
tweak to import logic for better support on python 3.10
Nospamas 8043590
tweak to how the sequence is updated to avoid interference from new n…
Nospamas ae64a61
enable and disable history triggers instead of removal
Nospamas 48d559d
Drop specific version default in favour of centralizing default versi…
Nospamas d9ace5c
Adjust mock tests so they're properly testing the correct function
Nospamas d736e48
blackify
Nospamas 9ef1ba9
Better error messages when we import an unexpected revision for tables
Nospamas 8f19e53
remove incorrect comment
Nospamas File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
247 changes: 247 additions & 0 deletions
247
pycds/alembic/versions/33179b5ae85a_add_network_key_column_to_meta_network.py
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,247 @@ | ||
| """add network_key column to meta_network | ||
|
|
||
| Revision ID: 33179b5ae85a | ||
| Revises: 8c05da87cb79 | ||
| Create Date: 2026-01-07 20:25:34.314026 | ||
|
|
||
| Notes: This process was made more complicated by some assumptions made by the history tracking code. | ||
| In particular it assumes that the primary and history table have the same column order with the | ||
| exception that history tables have additional columns at the end. When adding a new column it is added | ||
| at the end and therefore breaks the assumption. To work around this, we have to recreate the history table | ||
| with the correct column order. This involves renaming the existing history table, creating a new one with | ||
| the correct structure, copying the data over, and then dropping the old table. | ||
|
|
||
| This is needed because at the current time neither postgres nor alembic support adding a column at a specific | ||
| position in the table. | ||
|
|
||
| """ | ||
|
|
||
| from alembic import op | ||
| import sqlalchemy as sa | ||
| from sqlalchemy import text | ||
| from pycds.context import get_schema_name | ||
| from pycds.alembic.change_history_utils import ( | ||
| disable_primary_table_triggers, | ||
| create_history_table, | ||
| create_history_table_triggers, | ||
| create_history_table_indexes, | ||
| enable_primary_table_triggers, | ||
| ) | ||
| from pycds.alembic.util import grant_standard_table_privileges | ||
|
|
||
|
|
||
| # revision identifiers, used by Alembic. | ||
| revision = "33179b5ae85a" | ||
| down_revision = "8c05da87cb79" | ||
| branch_labels = None | ||
| depends_on = None | ||
|
|
||
| schema_name = get_schema_name() | ||
|
|
||
|
|
||
| def upgrade(): | ||
| # Create a function to generate network key from network name | ||
| # Replicates the behavior of Network.gen_key_from_name() in orm Tables.py | ||
| op.execute( | ||
| text( | ||
| f""" | ||
| CREATE OR REPLACE FUNCTION {schema_name}.gen_network_key_from_name(name text) | ||
| RETURNS text | ||
| LANGUAGE sql | ||
| IMMUTABLE | ||
| AS $$ | ||
| SELECT lower(replace(replace(trim(name), ' ', '_'), '-', '_')) | ||
| $$ | ||
| """ | ||
| ) | ||
| ) | ||
|
|
||
| # Disable existing triggers before modifying table structure so that we don't accidentally track | ||
| # the intermediate states | ||
| disable_primary_table_triggers("meta_network") | ||
|
|
||
| # Rename the existing history table to preserve existing history data | ||
| # We'll copy data from this into the new table with the correct column order | ||
| op.execute( | ||
| text(f"ALTER TABLE {schema_name}.meta_network_hx RENAME TO meta_network_hx_old") | ||
| ) | ||
|
|
||
| # Drop the identity property, which also drops the associated sequence | ||
| op.execute( | ||
| text( | ||
| f"ALTER TABLE {schema_name}.meta_network_hx_old ALTER COLUMN meta_network_hx_id DROP IDENTITY" | ||
| ) | ||
| ) | ||
|
|
||
| op.add_column( | ||
| "meta_network", | ||
| sa.Column( | ||
| "network_key", | ||
| sa.String(), | ||
| nullable=True, | ||
| ), | ||
| schema=schema_name, | ||
| ) | ||
|
|
||
| op.execute( | ||
| text( | ||
| f""" | ||
| UPDATE {schema_name}.meta_network | ||
| SET network_key = {schema_name}.gen_network_key_from_name(network_name) | ||
| """ | ||
| ) | ||
| ) | ||
|
|
||
| op.create_unique_constraint( | ||
| "uq_meta_network_network_key", | ||
| "meta_network", | ||
| ["network_key"], | ||
| schema=schema_name, | ||
| ) | ||
|
|
||
| # Create a trigger function to auto-populate network_key on INSERT. Must be a trigger as | ||
| # Deault values can't call functions that access other columns. | ||
| op.execute( | ||
| text( | ||
| f""" | ||
| CREATE OR REPLACE FUNCTION {schema_name}.set_network_key_default() | ||
| RETURNS trigger | ||
| LANGUAGE plpgsql | ||
| AS $$ | ||
| BEGIN | ||
| IF NEW.network_key IS NULL THEN | ||
| NEW.network_key := {schema_name}.gen_network_key_from_name(NEW.network_name); | ||
| END IF; | ||
| RETURN NEW; | ||
| END; | ||
| $$ | ||
| """ | ||
| ) | ||
| ) | ||
|
|
||
| # Create trigger to run before INSERT | ||
| op.execute( | ||
| text( | ||
| f""" | ||
| CREATE TRIGGER set_network_key_default_trigger | ||
| BEFORE INSERT ON {schema_name}.meta_network | ||
| FOR EACH ROW | ||
| EXECUTE FUNCTION {schema_name}.set_network_key_default() | ||
| """ | ||
| ) | ||
| ) | ||
|
|
||
| # Recreate the history table with the new column structure | ||
| create_history_table("meta_network", foreign_tables=None) | ||
| grant_standard_table_privileges(f"{schema_name}.meta_network_hx") | ||
|
|
||
| # Copy existing history data from the old table to the new one | ||
| op.execute( | ||
| text( | ||
| f""" | ||
| INSERT INTO {schema_name}.meta_network_hx | ||
| (network_id, network_name, description, virtual, | ||
| publish, col_hex, mod_time, mod_user, | ||
| network_key, deleted, meta_network_hx_id) | ||
| SELECT | ||
| network_id, network_name, description, virtual, | ||
| publish, col_hex, mod_time, mod_user, | ||
| {schema_name}.gen_network_key_from_name(network_name), | ||
| deleted, meta_network_hx_id | ||
| FROM {schema_name}.meta_network_hx_old | ||
| ORDER BY meta_network_hx_id | ||
| """ | ||
| ) | ||
| ) | ||
|
|
||
| # Update foreign key references in dependent tables to point to the new history table | ||
| # meta_station_hx and meta_vars_hx have foreign keys to meta_network_hx | ||
|
|
||
| # Drop the foreign key constraints from dependent tables | ||
| op.execute( | ||
| text( | ||
| f"ALTER TABLE {schema_name}.meta_station_hx DROP CONSTRAINT meta_station_hx_meta_network_hx_id_fkey" | ||
| ) | ||
| ) | ||
| op.execute( | ||
| text( | ||
| f"ALTER TABLE {schema_name}.meta_vars_hx DROP CONSTRAINT meta_vars_hx_meta_network_hx_id_fkey" | ||
| ) | ||
| ) | ||
|
|
||
| # Drop the old history table now that data has been copied and FKs removed | ||
| op.execute(text(f"DROP TABLE {schema_name}.meta_network_hx_old")) | ||
|
|
||
| # Set the sequence to continue from the next ID after the last copied record | ||
| op.execute( | ||
| text( | ||
| f""" | ||
| SELECT setval( | ||
| '{schema_name}.meta_network_hx_meta_network_hx_id_seq', | ||
| (SELECT COALESCE(MAX(meta_network_hx_id), 1) FROM {schema_name}.meta_network_hx), | ||
| true | ||
| ) | ||
| """ | ||
| ) | ||
| ) | ||
|
|
||
| # Recreate the foreign key constraints pointing to the new history table | ||
| op.execute( | ||
| text( | ||
| f""" | ||
| ALTER TABLE {schema_name}.meta_station_hx | ||
| ADD CONSTRAINT meta_station_hx_meta_network_hx_id_fkey | ||
| FOREIGN KEY (meta_network_hx_id) | ||
| REFERENCES {schema_name}.meta_network_hx(meta_network_hx_id) | ||
| """ | ||
| ) | ||
| ) | ||
| op.execute( | ||
| text( | ||
| f""" | ||
| ALTER TABLE {schema_name}.meta_vars_hx | ||
| ADD CONSTRAINT meta_vars_hx_meta_network_hx_id_fkey | ||
| FOREIGN KEY (meta_network_hx_id) | ||
| REFERENCES {schema_name}.meta_network_hx(meta_network_hx_id) | ||
| """ | ||
| ) | ||
| ) | ||
|
|
||
| # Recreate the history tracking triggers | ||
| enable_primary_table_triggers("meta_network") | ||
| create_history_table_triggers("meta_network", foreign_tables=None) | ||
|
|
||
| # Create indexes on the history table | ||
| create_history_table_indexes( | ||
| "meta_network", "network_id", foreign_tables=None, extras=None | ||
| ) | ||
|
|
||
|
|
||
| def downgrade(): | ||
| # Drop the trigger and trigger function | ||
| op.execute( | ||
| text( | ||
| f"DROP TRIGGER IF EXISTS set_network_key_default_trigger ON {schema_name}.meta_network" | ||
| ) | ||
| ) | ||
| op.execute(text(f"DROP FUNCTION IF EXISTS {schema_name}.set_network_key_default()")) | ||
|
|
||
| # Drop the constraint and column from primary table | ||
| op.drop_constraint( | ||
| "uq_meta_network_network_key", | ||
| "meta_network", | ||
| type_="unique", | ||
| schema=schema_name, | ||
| ) | ||
|
|
||
| # When dropping we don't have the same issues with column order so we can safely just drop the | ||
| # column to return to the pre-migration state | ||
| op.drop_column("meta_network", "network_key", schema=schema_name) | ||
|
|
||
| # Drop the column from history table | ||
| op.drop_column("meta_network_hx", "network_key", schema=schema_name) | ||
|
|
||
| # Drop the key generation function | ||
| op.execute( | ||
| text(f"DROP FUNCTION IF EXISTS {schema_name}.gen_network_key_from_name(text)") | ||
| ) | ||
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,39 @@ | ||
| """ | ||
| ORM declarations for tables. | ||
|
|
||
| This module provides access to table ORM definitions, with support for | ||
| retrieving definitions at specific migration revisions for testing purposes. | ||
|
|
||
| By default, imports from this module provide the current (head) version of tables. | ||
| Tests can set a specific revision using set_global_table_version() before importing. | ||
| """ | ||
|
|
||
| from pycds.orm.versioning import ( | ||
| get_global_table_version, | ||
| ) | ||
|
|
||
| # Check if a specific version has been requested | ||
| _requested_version = get_global_table_version() | ||
|
|
||
| if _requested_version is None: | ||
| # No specific version requested - import from (current/head) | ||
| from .version_33179b5ae85a import * | ||
| else: | ||
| # Specific version requested - import from that version module | ||
| import importlib | ||
| try: | ||
| _version_module = importlib.import_module( | ||
| f"pycds.orm.tables.version_{_requested_version}" | ||
| ) | ||
|
|
||
| # Import all public members from the version module | ||
| for _name in dir(_version_module): | ||
| if not _name.startswith("_"): | ||
| globals()[_name] = getattr(_version_module, _name) | ||
|
|
||
| del importlib, _version_module, _name | ||
| except ModuleNotFoundError as e: | ||
| raise ImportError( | ||
| f"Table version module for revision '{_requested_version}' not found. Ensure that " | ||
| f"the migration revision exists and that the corresponding version module has been created." | ||
| ) from e |
Oops, something went wrong.
Oops, something went wrong.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
To Quintin's point,
network_name, though currently NULLable, shouldn't be (and in practice, never is), and I thinknetwork_keyshould not be nullable either. What good is a key that's NULL :)There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I agree but I think I'm I'm stuck in a catch 22 here. I can't add a new column with out it being either a) nullable or b) have a default value applied. Default values in postgres need to be pretty simple and can't refer to other column data even via functions.
In order to circumvent this the code is as currently applied: The column is created as a nullable but a trigger will populate this column any time an insert happens acting as a default.