Add multi-card primary descriptor #1368

[jacobtylerwalls]

Description

Retires the primary descriptor from core arches that was limited to a single card. Creates a new primary descriptor that can draw from any card (using node aliases, which are unique), and wires it into the existing location for managing this (the function manager).

A migration deploys triggers to the tiles table that will calculate descriptors and writes to the resource instance table. (Another trigger captures changes to the function config itself and recalculates every resource.)

This migration was generated by django-pgtrigger, which abstracts away some of the boilerplate surrounding the trigger (e.g FOR EACH ROW etc), and allows for Django-y filters like Q() to add conditions on the trigger. This trigger is particularly complicated, but I can see this being helpful for simpler ones. Another benefit is managing the triggers in the application code and getting simple git diffs on the source.

Setup instructions

• depends on Fix function registration arches#10286 to register the function
• depends on Make primary descriptor fn aware of which description it's calculating #10299 arches#10300 to pass down the descriptor type
• depends on Add contenttypes dependency to initial migration #10302 arches#10303 to fix migration order failure
• in testing project, e.g. disco, add to settings like so:

FUNCTION_LOCATIONS.append('arches_for_science.pkg.extensions.functions')

• reinstall arches-for-science dependencies to get django-pgtrigger
• reload the afs package in testing project (e.g. disco)
• run migrations

Fixes #1368

[jacobtylerwalls]

These now need to be node aliases rather than node names. I could make the lookup case-insensitive, I suppose?

[chiatt]

That would work in many cases, but a node alias can differ from the node name in other ways, generally because an alias must be unique to a graph and a name only must be unique to its siblings. So, I think it's good to be explicit about changing from names to aliases as you've done here.

[chiatt]

That would work in many cases, but a node alias can differ from the node name in other ways, generally because an alias must be unique to a graph and a name only must be unique to its siblings. So, I think it's good to be explicit about changing from names to aliases as you've done here.

[chiatt]

I would expect the trigger to be removed on the reverse migration, but it persists. When you reverse migrate, are the pgtriggers dropped?

[jacobtylerwalls]

Works for me:

Reversing from 0008 to 0004:

root@d6b16aa0f3ba:/web_root/disco# python3 manage.py migrate arches_for_science 0004
System check identified some issues:

WARNINGS:
?: (urls.W005) URL namespace 'admin' isn't unique. You may not be able to reverse all URLs in this namespace
?: (urls.W005) URL namespace 'oauth2' isn't unique. You may not be able to reverse all URLs in this namespace
Operations to perform:
  Target specific migration: 0004_improve_primary_descriptor, from arches_for_science
Running migrations:
  Rendering model states... DONE
  Unapplying arches_for_science.0008_remove_tilemodelproxy_calculate_multicard_primary_descriptor_single_and_more... OK
  Unapplying arches_for_science.0007_update_primary_description_trigger_handle_static_value... OK
  Unapplying arches_for_science.0006_update_primary_description_trigger_null_handling... OK
  Unapplying arches_for_science.0005_deploy_triggers_for_primary_descriptor... OK
root@d6b16aa0f3ba:/web_root/disco# python3 manage.py dbshell
psql (14.9 (Ubuntu 14.9-1.pgdg22.04+1), server 14.5 (Debian 14.5-1.pgdg110+1))
Type "help" for help.

disco=# \dS tiles
...
Triggers:
    __arches_check_excess_tiles_trigger BEFORE INSERT ON tiles FOR EACH ROW EXECUTE FUNCTION __arches_check_excess_tiles_trigger_function()
    __arches_trg_update_spatial_attributes AFTER INSERT OR DELETE OR UPDATE ON tiles DEFERRABLE INITIALLY DEFERRED FOR EACH ROW EXECUTE FUNCTION __arches_trg_fnc_update_spatial_attributes()

I noticed that there was a missing migration on this branch from Friday, that might have been the issue.

[jacobtylerwalls]

@chiatt FYI -- during release testing I just discovered that node aliases are nullable. So this hasn't really been tested against that (I would guess that which node we resolve <> to is probably undefined).

[chiatt]

@chiatt FYI -- during release testing I just discovered that node aliases are nullable. So this hasn't really been tested against that (I would guess that which node we resolve <> to is probably undefined).

There must have been a reason for making them nullable when the alias was introduced. I don't recall what that would have been, but perhaps it should be changed - in 7.6 at least.

[chiatt]

Looks great!