CEXT-6511: [SPEC] Execute Custom Installation Steps During Application Upgrade - #628
CEXT-6511: [SPEC] Execute Custom Installation Steps During Application Upgrade#628jcuerdo wants to merge 3 commits into
Conversation
… CEXT-6511-spec
|
github-actions
Bot
added
without-changeset
obarcelonap
left a comment
obarcelonap
left a comment
There was a problem hiding this comment.
For the sake of simplicity, I'd go with A. It's easy to explain, automatically reconciles any drift that may have occurred, doesn't introduce additional version coordinates that need to be maintained manually, doesn't require persisting any associated state and code changes in the SDK are minimal.
We can always revisit this decision later if it turns out not to be sufficient.
| the last persisted value. Removed steps behave the same as in Option A. The extra state needed is | ||
| the last-run version per step name. | ||
|
|
||
| ### Option C: Migration-style steps (append-only) |
There was a problem hiding this comment.
If we go with A, this is something you can always build yourself by defining a custom step that delegates the work to a JavaScript migration framework.
We can explore that approach later and provide it as a sample implementation, or even develop a more opinionated abstraction on top of option A.
There was a problem hiding this comment.
The main difference here is that custom steps are code, not just configuration.
For example, if a webhook is removed from the config, the SDK already knows how to delete that webhook. But if a custom step is removed, the SDK also loses the reference to the script and its uninstall function. Comparing the old and new config can tell the SDK that the step was removed, but it cannot run uninstall if the old script is no longer included in the application. Custom steps therefore need a way to keep that old code available.
With that in mind, I think the main decision is between Options A and C. Option B seems mostly dominated by the other two: if the latest script must handle upgrades from every previous state, it is effectively Option A with additional version bookkeeping; if it depends on intermediate script versions having run, direct upgrades can fail because those versions are not preserved.
Option A is simpler, but requires every script to be safe to run repeatedly and able to bring the app from any previous state to the desired state. Option C keeps scripts as an ordered migration history, but requires every published script to remain available and unchanged, stable step identities, and a record of which steps have already run so that only new ones execute. I would discard Option B unless we identify a concrete use case that neither A nor C covers.
| that already ran run again, and under what condition? And if a step is no longer in the config, | ||
| should its `uninstall` run, and when? |
There was a problem hiding this comment.
The problem with this is that you have to keep track of where the uninstall code is, because it's deployed with the app itself, so that it can be executed. The only bridge between the SDK and the uninstall code is its explicit declaration in the app.commerce.config. Without that entry, we don't have a way to "find" and "invoke" that code.
The only way (that I see) for doing that is by making the app store the full history of the app.commerce.config file across versions. Something like:
src
commerce-extensibility-1/
.generated/
.history/
app.commerce.config-v1.0.0
app.commerce.config-v1.0.1
app.commerce.config-v1.0.2
// ...rest
Each version would then contain the install and uninstall code inlined (self-contained). Which would keep it invocable.
There was a problem hiding this comment.
Sidenote: Keeping this history solves other problems in a nice way, too:
- We always have baselines to compare for upgrade (this was also suggested by @asalloum5)
- Rolling back to a previous config is straightforward (you can even roll back several at a time)
But at the cost of making things slightly more fragile (Git is the only safeguard against losing a previous version)
| This adds a required field per step, and asks developers to bump `version` whenever the logic | ||
| changes in a way that needs to re-run. The SDK can't verify that, it only compares against | ||
| the last persisted value. Removed steps behave the same as in Option A. The extra state needed is | ||
| the last-run version per step name. |
There was a problem hiding this comment.
Another way to detect changes in custom installation steps is to hash the bodies of the install and uninstall and compare those hashes. We tested this with @obarcelonap, and it was indeed possible to do
| - Version bumps are self-reported and unverifiable: forget to bump it and a step silently skips; | ||
| bump it by mistake and it re-runs for no reason. |
There was a problem hiding this comment.
Comparing the hashes as I suggested above could cause a failure if the version is the same but hashes do not match.
| - A step only has one script body at a time. If 1.0.0 and 1.1.0 are meant to be two separate | ||
| actions, not 1.1.0 replacing 1.0.0 outright, there's no way to express that in a single versioned | ||
| file. A fresh install only ever runs whatever the current file contains, once. Getting this right | ||
| means the same idempotency discipline as Option A, just scoped to "since the last version bump" | ||
| instead of "always." |
There was a problem hiding this comment.
This seems like an important limitation of Option B. For example:
- In
1.0.1, stepupdateDatabaseversion1.1.0creates a new column. - In
1.0.2, the same step becomes version1.2.0and writes data to that column, assuming the1.1.0logic already ran.
An app upgrading directly from 1.0.0 to 1.0.2 would execute only the current 1.2.0 script. It would not execute the previous 1.1.0 script first, so the upgrade could fail because the column does not exist.
In other words, the version field tells us whether to run the current script, but it does not preserve or execute the intermediate versions. Avoiding this would require every new script version to handle upgrades from every supported previous state, which brings us back to requiring idempotent/reconciling scripts.
| means the same idempotency discipline as Option A, just scoped to "since the last version bump" | ||
| instead of "always." | ||
|
|
||
| **Option C:** |
There was a problem hiding this comment.
Option C is essentially a small-scale equivalent to keeping a history of app configurations as I proposed above. If we were going to do that I would rather go for keeping a history of the whole config, which also brings other benefits to the table
3 participants
Description
Related Issue
Motivation and Context
How Has This Been Tested?
Screenshots (if appropriate):
Types of changes
Checklist: