Database Packaging - #1121
Conversation
isc-jili
left a comment
There was a problem hiding this comment.
Looks great @isc-dchui ! I left some comments!
bba26d8 to
216223e
Compare
| // Identical to "package": route to the Package phase using current lifecycle | ||
| set tCommandInfo = "package" | ||
| do ..RunOnePhase(.tCommandInfo) | ||
| } elseif (tCommandInfo = "package-database") { |
There was a problem hiding this comment.
@isc-dchui I remember that adding package-** commands was part of your original design, but I can't remember why we went with that instead of flags on the existing package command so that it continues to be considered a module-action rather than separate commands. I'm not sure I have a strong preference either way, design-wise, but I'm noticing that the flow through the Shell is different.
There was a problem hiding this comment.
It's because of the lifecycle phases. A separate command allows it to dispatch to the %IPM.Lifecycle.Database class directly
isc-kiyer
left a comment
There was a problem hiding this comment.
@isc-dchui few small notes. I love the description of the MR! Could you add some more information for each of the "Key Details"? In particular, for the following:
- Can you explain why generated resources need special treatment? When packaging, wouldn't they be in the routine db already?
- What is the reason for exporting the module manifests and reimporting them? Could that not be accomplished by setting up a global mapping/copying over the IPM globals from the globals db to the routine db during packaging? Then its one less step at deployment time.
| // then it should have a package mapping of this specific resource to its namespace database | ||
| $$$ThrowOnError(..OnConfigureMappings(.pParams)) | ||
| } | ||
| if ..ResourceReference.Generated { |
There was a problem hiding this comment.
That's just dead code. If it's generated, it gets marked as handled at the bottom of the method anyway.
| <description> | ||
| Creates an IRIS.DAT database package bundled in a .tgz containing IRIS.DAT, module.xml (with SHA-256 checksum), and dependencies.xml. | ||
| </description> | ||
| <parameter name="module" required="true" description="Name of module to package as database" /> |
There was a problem hiding this comment.
Should indicate that this module should not be a dependent of any other module (something we should check and throw an error for). We should also probably check for DB packaging that every module other than this one has at least 1 dependent because if not, you are packaging things that aren't intended.
There was a problem hiding this comment.
Turns out this we don't need this constraint since database-packaging will iterate through the specified module's resources (and dependencies), so any module that depends on this one will simply be ignored.
|
|
||
| if useCurrentDB { | ||
| // -use-current-db: use the existing routines DB directly, no remapping | ||
| // Note: globals mapping is unchanged throughout — IPM metadata stays in globals DB |
There was a problem hiding this comment.
Wouldn't it be better to have IPM metadata be put into routine db? Or I guess we achieve this by exporting the module manifests and loading them into the ns after mounting the IRIS.DAT on install from db packaged?
There was a problem hiding this comment.
Right, the export/import is the mechanism and it avoids some trickiness around namespace/database mappings and possibly conflicting IPM data
| { | ||
| new $namespace | ||
| set $namespace = "%SYS" | ||
| $$$ThrowOnError(##class(Config.Namespaces).Get(ns, .nsProps)) |
There was a problem hiding this comment.
Nit: instead of needing a ns switch, we can use ##class(%SYS.Namespace).GetAllNSInfo()
There was a problem hiding this comment.
Right, but we only need it for one namespace so seemed unnecessary to get it for each
|
|
||
| // get metadata from annotations | ||
| set metadata = ..GetPackageMetadata(..Location, name, tag, "", client) | ||
| if (metadata = "") { |
There was a problem hiding this comment.
What is the metadata value if the tag doesn't exist in the tag list? So that it's not equal to ""
There was a problem hiding this comment.
GetAllTags() won't return it so that tag won't be part of the while loop.
set allTagsString = ..GetAllTags(..Location, name, "", client)
set allVersionsList = ..AggregatePlatformVersions($listfromstring(allTagsString, ", "), .aggregatedPlatformVersion)
set pointer = 0
while $listnext(allVersionsList,pointer,moduleVersion) {
...
}
1193db1 to
a366d71
Compare
@isc-kiyer Updated the description with a little more organization clarity! To explicitly answer these,
|
isc-jili
left a comment
There was a problem hiding this comment.
Looks good! @isc-dchui One more Q: are all the resources as part of transitive dependencies supposed to be packaged up too? Since ExportGeneratedResources() CopyNonCompiledResources() and ExportPythonDependencies() seem to be invoked on the root module only
A test case along those lines that would be nice also would be having a dependency contain a Generated="1" resource or a requirements.txt
| pVersion As %String, | ||
| pDeployed As %Boolean, | ||
| pPlatformVersion As %String, | ||
| pIPMPackaging As %String = "", |
There was a problem hiding this comment.
would adding it as param #6 mean that uses of this method would break?
There was a problem hiding this comment.
because currently param 6 is pParams and param 7 is pDedendencyGraph
There was a problem hiding this comment.
Yes should add to the end of the method for backwards compatibility
There was a problem hiding this comment.
it does look like all calls to this method are accounted for however, so nothing within IPM would break
There was a problem hiding this comment.
and as for our uses of IPM, we never call this method directly, there are other methods that wrap around this one that tend to be more user-entry-point friendly
There was a problem hiding this comment.
Right, I initially had it at the end but @isc-jlechtne suggested I move it so I did and made sure all its callers were also updated. It seems pretty unlikely that there would be an external caller of this method directly so it seemed safe enough, but also happy to revert it.
There was a problem hiding this comment.
Hmm taking a step back, if we are making a backwards incompatible change here, I propose just passing in the full module reference (as the method name suggests) instead of just passing specific properties. That also ensures any new stuff added to the module ref in future doesn't require changing the signature here
There was a problem hiding this comment.
Changed LoadModuleReference to take a %IPM.Storage.QualifiedModuleInfo oref as the first argument, replacing the various properties
…g to dependencies of db-pkg
07873a8 to
e654cba
Compare
|
Fixed the issue where, on install, dependencies wouldn't run through their lifecycle phases. Details are in a new section of the description. |
| } | ||
| // Nothing usable was written: an empty export means the resource matched no compiled | ||
| // classes, which is expected for resources this namespace never generated. | ||
| if ##class(%Library.File).Exists(tempFile) { |
There was a problem hiding this comment.
Nit: This seems like it belongs as another another elseif clause above
| do ..StageResourceFile(resource.Name, sourcePath, destPath) | ||
|
|
||
| } elseif exportPythonDeps && resource.Processor.%IsA("%IPM.ResourceProcessor.PythonWheel") { | ||
| set wheelDir = resource.Processor.Directory |
There was a problem hiding this comment.
Nit (Inconsistent): xDir and xName have opposite order in when they were set in this clause vs the next CPF one.
| continue | ||
| } | ||
|
|
||
| kill children |
| if $$$ISERR(tSC) { | ||
| quit | ||
| } | ||
| // Map ^IRIS.Msg to the namespace's own database so LOC compilation can write message domains. |
There was a problem hiding this comment.
For my understanding, why is this section needed for DB packaging?
|
|
||
| ### Added | ||
| - #1117: Add `sync` command for incremental loading of changed files in dev-mode modules. Detects modified files since last sync using SHA-1 hash and recompiles only what is stale. Supports `-delete` for processing removed files and `-test` for running changed test-phase unit tests. | ||
| - #986: Database packaging: new `package-database` and `publish-database` commands create an IRIS.DAT-based package that installs via swapping of the routines database rather than compilation of source files. Pass `-dev` to include test resources (`Scope="test"` or `Scope="verify"`) in the package, which are excluded by default. |
There was a problem hiding this comment.
Should the new command package-source (and maybe package-studio-project as well, though I see that's marked deprecated already) be listed here as well? Or did you decide that wasn't necessary due to their equivalence to the package command?
|
|
||
| set sc = ##class(%IPM.Main).Shell("load " _ tamperedPackage _ " -swap-db") | ||
| do $$$AssertStatusNotOK(sc, "Tampered IRIS.DAT rejected by checksum validation") | ||
| do $$$AssertTrue($system.Status.GetErrorText(sc) [ "SHA-256", "Error mentions SHA-256 checksum mismatch") |
There was a problem hiding this comment.
If we're checking for an error substring here, then the call before of checking the status not okay is redundant.
| set $namespace = installNS | ||
|
|
||
| set sc = ##class(%IPM.Main).Shell("load " _ missingDepsPackage _ " -swap-db") | ||
| do $$$AssertStatusNotOK(sc, "Missing deps/ directory rejected by validation") |
There was a problem hiding this comment.
same as above, no longer need this line
|
|
||
| set sc = ##class(%IPM.Main).Shell("load " _ noChecksumPackage _ " -swap-db") | ||
| do $$$AssertStatusNotOK(sc, "module.xml without <Checksum> rejected by validation") | ||
| do $$$AssertTrue($system.Status.GetErrorText(sc) [ "Checksum", "Error mentions the missing <Checksum> element") |
There was a problem hiding this comment.
same as above, don't need to check that there was an error if we're also making a check for the error text
| // Attempt to install second database package should fail | ||
| set sc = ##class(%IPM.Main).Shell("load " _ dbPackage2 _ " -swap-db") | ||
| do $$$AssertStatusNotOK(sc, "Cannot install second database package in same namespace") | ||
| do $$$AssertTrue($system.Status.GetErrorText(sc) [ "Only one database package", "Error mentions one-package-per-namespace constraint") |
There was a problem hiding this comment.
same as above, no need to check that there was an error if we're also making a check for the error text
| // --- Cleanup --- | ||
| // Unpublish all remaining versions of dep-module and main-with-deps from zot | ||
| set $namespace = packagingNS | ||
| set sc = ##class(%IPM.Main).Shell("unpublish zot/dep-module all -f") |
There was a problem hiding this comment.
Don't need to set a status here since it's never used
isc-jlechtne
left a comment
There was a problem hiding this comment.
Looking great! Left some comments
Description
Resolves #986
Overview
Packaging workflow
Running
package-database <module>(orpublish-database <module>) forces the%IPM.Lifecycle.Databaselifecycle class and routes to the Package (or Publish) phase. The full pipeline inside%Package:%IPM.Main.CLSmust not live in the namespace's local routines DB, since that DB is about to be dismounted. Also validates the routines DB is not mirrored (dismounting a mirrored DB breaks the mirror).Generated=trueresources from the source namespace to temp XML files before remapping. These are classes whose compiled form exists in the source DB but whose generator does not run during standard%Reload/%Compile— for example, a class produced by an external tool. Once the old DB is dismounted, compiled generated classes are inaccessible. Resources that do regenerate during packaging (e.g. BPL/DTL produce a.1.clssubclass during compilation) are exported and re-imported harmlessly — the import is a no-op since the class already exists in the temp DB by then.-use-current-db). The remap happens before dismounting the old DB to avoid any window where the namespace has no mounted routines DB.<Packaging>database</Packaging>and a SHA-256<Checksum>), adeps/directory of per-dependency IRIS export XML files, and any non-compiled resources (wheels, FileCopy, CPF). Create the.tgzfrom the staging directory.If an error occurs after the remap (step 3), a safety net in
%Packagerestores the namespace before propagating the error.For OCI registries, database packages are tagged as
<version>_database__<IRIS-major>.<IRIS-minor>and the manifest carries acom.intersystems.ipm.packaging=databaseannotation. Source packages use existing tag formats with no annotation (backwards compatible — absent annotation means source).When publishing,
Base.%Publishcalls..Package(.pParams, 0)first (populating..Payload), then uploads to the registry. The lifecycle class'sPACKAGINGparameter determines the tag format, not the module's storedPackagingproperty (which reflects how the module was loaded and may be stale).Installation workflow
From a registry (
install/update): the-swap-dbmodifier controls which packaging type is resolved:-swap-db: only source-packaged modules are fetched. If none exist but a database package does, the user is told to re-run with-swap-db.-swap-db: only database-packaged modules are fetched. Dependencies are always resolved as source packages regardless of the top-level flag, since a database package already embeds its dependencies in the IRIS.DAT.From a local path (
load -path <tgz>): no packaging filter is applied.loadpasses all command data (includingSwapDBif-swap-dbwas given) through to the lifecycle. Without-swap-db,DoDatabaseInstallprompts the user for confirmation before proceeding.IsInstallContext()returns true whenmodule.Packaging = "database"andIRIS.DATis present in the module root. When true,%Reloadroutes toDoDatabaseInstall, which:databasepackaging,deps/directory exists, SHA-256 checksum matches.-swap-dbwas not passed. The prompt lists any modules that will be removed by the swap.IRIS_<dbname>_<timestamp>.DATas a backup → rename packaged IRIS.DAT into place → remount. Renames are atomic and avoid copying large files. On failure after the swap starts,RollbackDatabaseSwapattempts to restore the backup (best-effort, never throws — called from a catch block).deps/manifests via$system.OBJ.Load. The code is already in the swapped-in DB; only IPM metadata records need registering.updateonly runs steps introduced after the installed version. Skipped whenparams("Update")=1.Because all install work completes inside
%Reload,%Validate,%Compile, and%Activateare no-ops during install (they checkparams("IsInstallContext")). Invokes fire naturally after%Reloadreturns — the DB is mounted by then.Non-compiled resources (FileCopy, WebApplication, CPF) are handled by resource processor hooks at the Module level. They run via
OnBeforePhase/OnAfterPhaseindependently of the%Activateoverride.Notable details
package-sourcecommand added. Explicit alias for the existingpackagebehavior. Useful for clarity when both packaging types are in play.<Packaging>.Storage.ModulecallsBase.GetBaseClassForPackagingto derive the correct lifecycle class frommodule.xml's<Packaging>value rather than rejecting a mismatch.package-database. Unlikepackage(which defaults off),package-databasedefaultsExportPythonDependencies=1. Both requirements.txt-based wheels and explicit<PythonWheel>resources are staged into the.tgz. A missing wheels directory only warns if arequirements.txtexists — a module with no Python requirements legitimately has no wheels directory.InjectDatabasePackagingTransform(XData) injects<Packaging>and<Checksum>into module.xml. The checksum placeholderREPLACECHECKSUMis substituted in ObjectScript before the stylesheet runs — XSLT 1.0 has no parameterized element content.deps/is a directory, not a single file. Each dependency gets its own IRIS export XML, one per transitive dependency, loadable with$system.OBJ.Load.Testing
All tests are in
Test.PM.Integration.DatabasePackaging. The test infrastructure uses two shared lazy-initialized namespaces to avoid ~15 namespace create/teardown cycles per run:Per-test install namespaces are created fresh and torn down in OnAfterOneTest.
load -swap-db→ classes callable, module in list and historylist-include-test-resourcesincludes Scope=test (TestHelper) and UnitTest (Test) classes; default packaging excludes bothdeps/directory each fail validationupdate -path -swap-db; Packaging changes to "database"; backup confirms DB swap occurredlist; backup preserved; compiled classes gone from namespace-export-python-deps 0; lune importable after install. module-with-requirements: wheel included by default, excluded with flag<Invoke After="Compile">skipped during database install;<Invoke After="Activate">runs; global markers confirm each%Activateoverride-swap-db) → Packaging=module; database install (-swap-db) → Packaging=database; main-with-deps install resolves dep as source; after unpublishing source tag, install without-swap-dbfails with hint to use-swap-dbpackage-databaseoverwrites a stale IRIS version in<SystemRequirements>with the current versionPackaging=databaseand no explicit LifecycleClass derives%IPM.Lifecycle.Databaseon validation.tgz(exercises-use-current-dbcopy path)ComputeSHA256Hexproduces a 64-character lowercase hex stringNot Yet Handled
Dependency Lifecycle Support (Commit a8733ae and onwards)
Problem
When installing a database-packaged module, dependency code is compiled into the IRIS.DAT at packaging time and arrives already mounted after the swap. But the dependency modules' lifecycle phases (
Activate, CPF merges, etc.) never ran, so dependencies with resources like CPF entries, Python wheels, or data files had their code present but none of their install-time setup applied.Dependency Tree (example)
Topo (leaf-first) order:
leaf-dep-A → leaf-dep-B → parent-dep → main-module.Phase Execution: Root vs Dependencies
Each column is a module; rows are execution time (top = earlier). Dependency phases run in topological (leaf-first) order.
[1] — Root's
Initializefires pre-swap against the old DB. DepInitializeruns post-swap because their code is already in the mounted IRIS.DAT.[1'] —
ReassertInitre-runs root's Initialize-phase resources (CPF merges, Python wheels) after all deps so root's versions land last. Deps'Initializeruns post-swap and writes into the same global state root already set pre-swap; re-running root's restores the intended ordering.[2] —
ApplyUpdateStepsruns per-module only on updates (zpm "update"). On a fresh install,Activateseeds all update steps as already-run so a futureupdateonly executes steps added after this version.Dep phases are derived from the canonical load/install chain ending at
Activate(orApplyUpdateStepson updates), withReload,Compile, and*excluded because dep code is already compiled into IRIS.DAT.Rollback Timing
All steps below execute inside
%ReloadviaDoDatabaseInstall. The swap is the point of no return: after it commits, rollback is not attempted and failures are reported forward.Why the boundary sits where it does:
UnconfigureStaleModulesside effects (web app deletions, file removals) have no clean undo, but IRIS.DAT is untouched so a re-run from scratch is still possible.PerformDatabaseSwapis a filesystem rename, soRollbackDatabaseSwapcan reverse it. This is the last step with a clean undo path.listwill then show the previous version's metadata against the newly mounted code. Re-run the install to fix this; do not uninstall first, as it would delete resources by the old version's names.Other Notable Changes
Generated resources extended to deps.
ExportGeneratedResourcesnow iterates all transitive dependencies viaGetSelfAndDependencyModules(), not just the root module. Dep generated resources become inaccessible after the namespace routines DB is remapped, so they must be exported before the remap alongside root's.Ensemble opt-in for test namespaces.
%IPM.Test.Utils.CreateNamespacepreviously ensemble-enabled every test namespace unconditionally, adding setup cost even when tests don't need it. The method now acceptspEnableEnsemble(default 0). When set, it passespFromInstall=1to suppress HS-specific global mappings that would otherwise point^IRIS.Msgat HSLIB (read-only on IRIS for Health), then explicitly maps^IRIS.Msg/^IRIS.MsgNamesto the namespace's own database.DeleteNamespacewas also fixed to skip ensemble secondary and temp databases when the namespace was never ensemble-enabled.Warnings at packaging time. Two methods fire during
%Packageto surface problems early:WarnUnrunnableInvokes— flags<Invoke>elements on phases that never execute during a database install (root:InitializeandReload.Before; deps: the excluded phasesReload,Compile,*).WarnMappingsNotApplied— flags<Mapping>elements in any module. Mappings are applied byBase.%Reload, which this path bypasses. Applying them inline would deadlock against a concurrent<CPF>merge becauseConfig.Map*locks are held until the install transaction commits, andiris mergecannot wait that out.Packaged-manifest guard.
%Reloadnow rejects a source load from a package directory whose IRIS.DAT was already moved by a prior swap, rather than silently compiling source into the swapped-in database.New tests. Integration tests added:
TestDependencyResourcesAppliedDuringInstall(CPF, wheel, and data file resources on a dep are applied post-swap),TestNonCompiledResourcesAppliedDuringInstall(FileCopy and web app resources),TestInvokeBehaviorDuringDatabaseInstall(invokes fire at the right phases), andTestUpdateStepsWithDatabasePackaging(update steps run per-module on upgrade). New test fixturesdep-with-resourcesandmain-with-resource-depssupport these. Unit tests inTest.PM.Unit.LifecycleDatabasecover topological ordering, phase list correctness (excluded phases, canonical chain coverage), packaged-manifest detection, and SHA-256 format.Checklist
mainbranch rebased or merged.zpm test -only) and integration tests (zpm verify -only) pass.