Merge pull request #194 from pacificclimate/i-180-crmp_network_geoserver

Update view CrmpNetworkGeoserver with variable tags column

Author: rod-glover

README.md

@@ -19,16 +19,19 @@ With this package, one can recreate the database schema in a [PostgreSQL](http:/
 - [ORM contents and usage](docs/orm.md)
 - Database operations with Alembic
   - [Introduction](docs/database-operations/introduction.md)
-  - [Creating a new migration](docs/database-operations/create-new-migration.md)
   - [Applying a migration: Upgrade](docs/database-operations/migrate-upgrade.md)
   - [Applying a migration: Downgrade](docs/database-operations/migrate-downgrade.md)
   - [Creating a new PyCDS database](docs/database-operations/create-new-db.md)
-  - [Creating the initial migration](docs/database-operations/create-initial-migration.md)
 - Testing
     - [Project unit tests](docs/testing/project-unit-tests.md)
     - [Test migrations with a test database](docs/testing/test-migrations.md)
     - [Unit tests in client code](docs/testing/unit-tests-in-client-code.md)
-- Development notes
+- Development
+  - Migrations
+    - [Introduction](docs/dev-notes/migrations/introduction.md)
+    - [IMPORTANT ADVICE AND GUIDELINES](docs/dev-notes/migrations/important-notes.md)
+    - [Creating a new migration](docs/dev-notes/migrations/create-new-migration.md)
+    - [Creating the initial migration](docs/dev-notes/migrations/create-initial-migration.md)
   - [Creating and using SQLAlchemy extensions](docs/dev-notes/sqlalchemy-extensions.md)
   - [Creating and using Alembic extensions](docs/dev-notes/alembic-extensions.md)
 

docs/database-operations/create-new-migration.md

@@ -0,0 +1,99 @@
+# Creating a new migration
+
+After the PyCDS ORM has been modified, you will usually want to create an Alembic migration script to enable existing databases to be upgraded to the new model.
+
+Alembic can generate a migration script for you. It can generate an empty script, or autogenerate a script containing changes inferred by comparison between the ORM and an existing database. The command for this is `alembic revision ...`.
+
+In the context of PCIC's databases (CRMP, Metnorth), autogeneration is less useful than one might hope. PCIC's databases have not been managed solely through PyCDS, and so diverge from the ORM definitions. Much of this divergence has been cleaned up, but there are still differences in some table definitions, indexes and constraints. Furthermore, the autogenerate function is only aware of tables and table-related objects. It does not handle functions, views, and matviews, and these are critical to our databases' operation. 
+
+In this situation, autogenerate generates draft migrations containing a great deal of spurious and undesirable content, and also omits all content related to non-table objects. Fixing this actually adds unnecessary work.
+
+Therefore, developers usually create a new migration _without_ using the `--autogenerate` flag. The result is a skeleton migration script with empty `upgrade` and `downgrade` functions. The benefits are that it generates a unique revision identifier, creates an appropriately named file in the correct directory, and inserts the migration into the migration sequence automatically. See [Create a Migration Script](https://alembic.sqlalchemy.org/en/latest/tutorial.html#create-a-migration-script) for an example.
+
+## Generating a new migration script (without autogeneration)
+
+Instructions:
+
+1. Generate the migration script:
+
+    ```shell script
+    alembic revision -m "<message>"
+    ```
+
+   Notes:
+   - `PYCDS_SU_ROLE_NAME` is not required for this operation.
+   - The `<message>` should be a succinct description of what the migration accomplishes; for example "Add name column to Users". This message becomes part of the name and content of the script.
+
+2. Alembic writes a new script to the directory `pycds/alembic/versions`. Its name includes a unique revision identifier (a SHA) and a version of `<message>`.
+
+3. Add code to create, modify, and/or drop commands for all items to be managed in this migration. (These commands are on the object `alembic.op`, which is imported in the skeleton script.)
+
+   1. **Ensure that all changes respect the specified schema name** that the user will supply when this migration is applied. In particular:
+      1. It's useful to obtain the specified schema name with `schema=pycds.get_schema_name()`.
+      2. Ensure the specified schema name is used for all objects, including functions, views and materialized views. This should come without special effort due to the structure of how these items are declared, but it is worth verifying.
+
+   2. Do this for both upgrade and downgrade functions in the script!
+   3. Data and schema migrations are not handled separately and should be included as part of the migration where applicable.
+
+4. Write some tests for the migration, including tests for the data migration where applicable. Examples can be found in the existing code.
+
+5. Commit the new migration script and its tests to the repo.
+
+For more information, see [Create a Migration Script](https://alembic.sqlalchemy.org/en/latest/tutorial.html#create-a-migration-script).
+
+Modify the script to perform the necessary actions for upgrading and downgrading a database to/from this revision. For useful examples, see other scripts in directory `pycds/alembic/versions`. 
+
+## Autogenerating a new migration script
+
+As noted above, **_autogeneration is not usually helpful_**. However, here are some instructions if you wish to do so. For more details, see the [Alembic documentation](https://alembic.sqlalchemy.org/en/latest/autogenerate.html).
+
+To autogenerate a migration script, you must have a reference database schema for Alembic to compare to the modified PyCDS ORM. After Alembic autogenerates the script, you must edit it to ensure completeness, correctness, and that it respects the specified schema name (see instructions below).
+
+Instructions:
+
+1. Choose or create a reference database schema that is at the latest migration. (Or a database schema at an otherwise desired "base" migration, which is unusual but not necessarily wrong.)
+   To serve as a reference, a schema need not contain any data.
+
+1. If the reference database is not named in `alembic.ini`, create an entry there for it of the form
+
+   ```ini
+   [<db-label>]
+   sqlalchemy.url = postgresql://<user-name>@<server-name>/<db-name>
+   ```
+
+1. Autogenerate the migration script:
+
+    ```shell script
+    [PYCDS_SCHEMA_NAME=<schema name>] alembic -x db=<db-label> revision --autogenerate -m "<message>"
+    ```
+
+   Notes:
+   - `PYCDS_SU_ROLE_NAME` is not required for this operation.
+   - The `<message>` should be a succinct description of what the migration accomplishes; for example "Add name column to Users". This message becomes part of the name and content of the script.
+
+1. Alembic writes a new script to the directory `alembic/versions`. Its name includes a unique revision identifier (a SHA) and a version of `<message>`.
+
+1. Review and edit the script to ensure correctness, completeness, and that it respects the specified schema name. Specifically:
+
+   1. There are (still) significant differences between the CRMP schema and this ORM. Expect to remove or alter many of the autogenerated operations.
+
+   2. Review to ensure that it picks up all changes to tables and implements them appropriately.
+      In particular, Alembic does not pick up on changes of table or column name, which must be manually converted from "drop old name, add new name" to "rename". Some other schema changes are also not detected.
+
+   3. For more information, see [What does Autogenerate Detect (and what does it not detect?)](https://alembic.sqlalchemy.org/en/latest/autogenerate.html#what-does-autogenerate-detect-and-what-does-it-not-detect).
+
+   4. Manually add creation or dropping of the following things not handled by Alembic autogenerate:
+      1. Functions
+      1. Views
+      1. Materialized views
+
+   5. **Ensure that all changes respect the specified schema name** that the user will supply when this migration is applied. In particular:
+      1. Replace any autogenerated schema name usage (e.g., `schema='crmp'`) with the specified schema name (`schema=pycds.get_schema_name()`).
+      1. Ensure the specified schema name is used for functions, views and materialized views. This should come without special effort due to the structure of how these items are declared, but it is worth verifying.
+
+   6. Do this for both upgrade and downgrade functions in the script!
+
+1. Write some tests for the migration. Examples can be found in the existing code.
+
+1. Commit the new migration script and its tests to the repo.
+

docs/database-operations/create-initial-migration.md docs/dev-notes/migrations/create-initial-migration.md

@@ -0,0 +1,35 @@
+# IMPORTANT ADVICE AND GUIDELINES
+
+As noted elsewhere, creating a migration is not in principle very complicated. But in this project, where we manage many non-table objects, there are complications. We try to document those complications here.
+
+## Order of activity
+
+1. Replaceable objects require multiple versions to be retained under the revision identifier (see notes below). So the first step is usually to create a new skeleton migration from which the revision identifier can be obtained.
+
+2. Modify the ORM to reflect the changes to be implemented. This can include:
+
+   - Modifying a table (e.g., column definitions)
+   - Creating a new version of a replaceable object to be created or updated (e.g., a function, view or matview)
+
+3. Complete the migration script using the newly defined objects. See the next sections for some important advice on doing that.
+
+## Multiple versions of the same replaceable object
+
+For each migration script to work in perpetuity without error, **_all_** versions of replaceable objects must be retained in the codebase. A given migration will use only one or perhaps two distinct versions of a replaceable object, but there may be many such versions corresponding to many revisions (and attendant migrations). 
+
+For example, a view or materialized view may acquire new columns and have the query defining its columns updated accordingly. But because earlier migrations involving that matview depend on earlier versions of that object, those versions of the matview must also be retained.
+
+### Locations of versions of replaceable objects
+
+All object definitions are defined in the module `pycds.orm`. This module is further subdivided into object types such as tables (`pycds.orm.tables`), views (`pycds.orm.views`), etc. Whereas tables are mutated and do not need to have old versions retained, replaceable objects require all versions to be retained separately. Within each replaceable object type module (e.g., `pycds.orm.views`), there are separate version modules named according to the version in which one or more replaceable objects in that version is (re)defined (e.g., `pycds.orm.views.version_bb2a222a1d4a`).
+
+(This organization is a little awkward in practice, but logical. Other and possibly better ways of organizing things are conceivable.)
+
+## Obtaining the correct version of replaceable objects in migrations
+
+Recall that a replaceable object (a.k.a. non-table object) is one that cannot be modified in-place in the way that a table can. Instead (prompting the name), such an object must be replaced in its entirety.
+
+**In a migration script that updates a replaceable object, it is _essential_ to obtain the old and new replaceable objects directly from the version directories where they are defined.** This ensures that the correct version is dropped and/or created. 
+
+When dependent objects must be dropped and recreated in the course of updating an primary object, ensure you get each dependent object from the correct version directory, and not from the top-level `pycds` module. If you do not do this, migrations will fail or potentially update the database with the wrong objects.
+

docs/dev-notes/migrations/create-new-migration.md

@@ -0,0 +1,32 @@
+# Introduction
+
+## Essential references
+
+The Alembic documentation is our fundamental reference. Note: The documentation appears only to be available for the latest version of Alembic (1.13 at time of writing), which is significantly ahead of the version in use in this project (1.6 atow). It must be read with this in mind; wherever things do not work as documented, this may be the answer. Examining the code may be a way to clarify issues.
+
+Here is a list of key pages in the Alembic documentation:
+
+- [Alembic documentation](https://alembic.sqlalchemy.org/en/latest/index.html)
+  - [Alembic tutorial](https://alembic.sqlalchemy.org/en/latest/tutorial.html)
+  - [Alembic cookbook](https://alembic.sqlalchemy.org/en/latest/cookbook.html)
+  - [Operation reference](https://alembic.sqlalchemy.org/en/latest/ops.html)
+
+## Existing Alembic infrastructure in project
+
+When you have installed this project locally:
+
+- Alembic is installed. You can use Alembic commands on the command line.
+- The Alembic [migration environment](https://alembic.sqlalchemy.org/en/latest/tutorial.html#the-migration-environment) has been created and customized.
+- Alembic-specific content is in the directory `pycds/alembic`, following standard Alembic practice.
+- There are a number of already-written migration scripts in `pycds/alembic/versions`.
+- Extensions to support non-table objects (e.g., functions, views, matviews) have already been written. 
+  - Extensions to Alembic proper are in `pycds/alembic/extensions`.
+  - Extensions to SQLAlchemy are in `pycds/sqlalchemy`. These support the Alembic extensions, although they can be and are used elsewhere too.
+
+## Development activities
+
+In the abstract, creating a migration is a relatively straightforward activity. This project, however, is fairly complicated, and writing migrations is correspondingly more complicated. 
+
+In particular, managing non-table objects (functions, views, matviews, etc.) is not supported by Alembic out of the box. We have written extensions to support migrations involving them. Additionally, such objects are not mutable, and so must be dropped and replaced in their entirety -- unlike tables, in which elements (e.g., columns) can be modified in-place.
+
+Such non-table objects are called here _replaceable objects_. 

docs/dev-notes/migrations/important-notes.md

@@ -15,7 +15,7 @@
 from pycds.orm.views.version_22819129a609 import (
     CollapsedVariables as CollapsedVariablesView,
 )
-from pycds import CrmpNetworkGeoserver
+from pycds.orm.views.version_84b7fc2596d5 import CrmpNetworkGeoserver
 
 
 # revision identifiers, used by Alembic.

docs/dev-notes/migrations/introduction.md

@@ -0,0 +1,45 @@
+"""update CrmpNetworkGeoserver
+
+Revision ID: 6cb393f711c3
+Revises: fecff1a73d7e
+Create Date: 2024-01-05 13:06:02.811787
+
+"""
+import logging
+from alembic import op
+from pycds import get_schema_name
+from pycds.orm.views.version_84b7fc2596d5 import (
+    CrmpNetworkGeoserver as OldCrmpNetworkGeoserver,
+)
+from pycds.orm.views.version_6cb393f711c3 import (
+    CrmpNetworkGeoserver as NewCrmpNetworkGeoserver,
+)
+
+
+# revision identifiers, used by Alembic.
+revision = "6cb393f711c3"
+down_revision = "fecff1a73d7e"
+branch_labels = None
+depends_on = None
+
+
+logger = logging.getLogger("alembic")
+schema_name = get_schema_name()
+
+
+def drop_view(view):
+    op.drop_replaceable_object(view, schema=schema_name)
+
+
+def create_view(view):
+    op.create_replaceable_object(view, schema=schema_name)
+
+
+def upgrade():
+    drop_view(OldCrmpNetworkGeoserver)
+    create_view(NewCrmpNetworkGeoserver)
+
+
+def downgrade():
+    drop_view(NewCrmpNetworkGeoserver)
+    create_view(OldCrmpNetworkGeoserver)

pycds/alembic/versions/22819129a609_convert_collapsed_vars_mv_to_native_.py

@@ -8,15 +8,18 @@
 import logging
 from alembic import op
 import sqlalchemy as sa
-from sqlalchemy.dialects import postgresql
 from pycds import get_schema_name
 from pycds.orm.native_matviews.version_bf366199f463 import (
     StationObservationStats as StationObservationStatsMatview,
 )
 from pycds.orm.views.version_bf366199f463 import (
     StationObservationStats as StationObservationStatsView,
 )
-from pycds import CrmpNetworkGeoserver
+
+# Important: We must obtain replaceable database objects from the appropriate revision
+# of the database. Otherwise, later migrations will cause errors in earlier migrations
+# due to a mismatch between the expected version and the latest (head) version.
+from pycds.orm.views.version_84b7fc2596d5 import CrmpNetworkGeoserver
 
 # revision identifiers, used by Alembic.
 revision = "bf366199f463"
@@ -30,12 +33,10 @@
 
 
 def drop_dependent_objects():
-    """What it says on the box"""
     op.drop_replaceable_object(CrmpNetworkGeoserver)
 
 
 def create_dependent_objects():
-    """What it says on the box"""
     op.create_replaceable_object(CrmpNetworkGeoserver)
 
 

pycds/alembic/versions/6cb393f711c3_update_crmpnetworkgeoserver.py

@@ -5,7 +5,7 @@
 
 
 def check_migration_version(
-    executor, schema_name=get_schema_name(), version="fecff1a73d7e"
+    executor, schema_name=get_schema_name(), version="6cb393f711c3"
 ):
     """Check that the migration version of the database schema is compatible
     with the current version of this package.