Hybrid attribute migration phase 1: schema + dual-write

[knoxsp]

Summary

This PR implements phase 1 of the hybrid attribute migration in hydra-base while preserving resource_attr_id as the critical structural key.

Changes

• Additive schema migration for tResourceAttr and tTypeAttr:
  • add attr_name
  • add dimension_id
  • backfill values from tAttr
  • make attr_id nullable for transition
• Model updates for new hybrid fields on ResourceAttr and TypeAttr
• Dual-write support in resource-attribute creation paths:
  • populate attr_name and dimension_id when attr_id is provided
  • keep legacy attr_id behavior intact

Safety

• resource_attr_id is unchanged and remains the key used by scenarios/data links.
• Migration is additive and transitional; existing clients continue to work.

Validation

• Syntax checks on touched files
• No reported diagnostics in modified files

[knoxsp]

Code Review

This PR adds attr_name and dimension_id denormalized columns to tResourceAttr and tTypeAttr, backfills them from tAttr, makes attr_id nullable, and updates all add_attribute call sites to populate the new fields in a dual-write pattern. The migration and model changes are structurally sound for phase 1, but three forward-looking architectural gaps — all stemming from using attr_id=None as a dict key — will cause silent data loss when phase 2 introduces attr-id-less records, and the migration's downgrade is broken if any NULL rows exist.

---

Findings (most severe first)

hydra_base/lib/attributes.py:909 — network_ra_lookup key collision silently drops hybrid RAs

Two distinct hybrid ResourceAttrs on the same node both hash to key (None, 'NODE', X). The second is skipped as a "duplicate" even though it has a different attr_name, losing one insert with no error.

network_ra_lookup = {(ra.attr_id, ra.ref_key, _get_resource_id(ra)): ra for ra in network_resource_attributes}

hydra_base/lib/attributes.py:988 — inserted_ids return dict collapses all hybrid RAs for a resource to one entry

inserted_ids[(obj.get_resource_id(), None)] is overwritten for each subsequent hybrid RA on the same resource, so only the last inserted ID is tracked. Callers that depend on this mapping to wire scenario data will reference the wrong row.

inserted_ids[(obj.get_resource_id(), obj.attr_id)] = obj.id

hydra_base/db/model/network/resourceattr.py:30 and hydra_base/db/model/template.py:475 — Existing UniqueConstraints stop enforcing uniqueness once attr_id is NULL

SQL NULL != NULL, so UniqueConstraint('node_id', 'attr_id') and UniqueConstraint('type_id', 'attr_id') allow unlimited (node_id=X, attr_id=NULL) / (type_id=X, attr_id=NULL) rows. No replacement constraint on (node_id, attr_name) or (type_id, attr_name) is added — every code path that should be idempotent can silently accumulate duplicates, and downstream code iterating type_i.typeattrs will process them twice.

hydra_base/db/alembic/versions/9b9f7d7a4f21_hybrid_attr_phase1.py:65 — Downgrade makes attr_id NOT NULL without first clearing NULL rows

def downgrade():
    op.alter_column('tTypeAttr', 'attr_id', existing_type=sa.Integer(), nullable=False)
    op.alter_column('tResourceAttr', 'attr_id', existing_type=sa.Integer(), nullable=False)

This is the first step in the downgrade. PostgreSQL raises "column contains null values" and MySQL fails similarly if any attr_id=NULL row exists. The downgrade needs a DELETE or backfill before the ALTER.

hydra_base/lib/attributes.py:1111 — add_resource_attrs_from_type dedup check fails for hybrid typeattrs

attrs.get(item.attr_id) where item.attr_id=None always returns None because attrs is keyed by the attr_id of existing resource attrs. Every call will re-add every hybrid typeattr's resource attribute, causing duplicate inserts on repeated calls to the same type+resource.

attrs = {}
for res_attr in resource_attrs:
    attrs[res_attr.attr_id] = res_attr  # attrs[None] = last null-attr_id RA only

for item in type_i.typeattrs:
    if attrs.get(item.attr_id) is None:  # always None for hybrid typeattrs
        ra = resource_i.add_attribute(item.attr_id, ...)

hydra_base/lib/attributes.py:993 — Cache update crashes on cold cache: None + list TypeError

cache.get(f'network_resource_attributes_{network_id}') returns None on a cache miss, making None + [JSONObject(obj) for obj in objs] raise TypeError. The inserts have already flushed so the transaction is committed but the cache is left stale and the caller gets an exception. Pre-existing bug, but the PR routes more traffic through this path.

cache.set(f'network_resource_attributes_{network_id}',
    cache.get(f'network_resource_attributes_{network_id}') + [JSONObject(obj) for obj in objs], 60*60)

hydra_base/lib/attributes.py:634 — a.name.lower() raises AttributeError for attrs with name=None

The guard if a is not None checks the attr object but not its name field. A caller passing a partially constructed attr with name=None gets an unhandled AttributeError rather than a proper validation error.

unique_lower_names = list({a.name.lower() for a in attrs if a is not None})

---

Review generated by Claude Code