-
-
Notifications
You must be signed in to change notification settings - Fork 1.7k
build(craft): Update target execution order #4171
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
The order of targets Craft runs in a release matters. If a target fails, Craft exits and stops the release, not running the targets that come after the failed one. For this reason, targets that are easy to undo and don't have big consequences should come first, and those that are harder to undo and have more impact should run last; i.e. targets are sorted by ascending impact/cost of reverting ratio. Based on that guideline, this is the new order: 1. `aws-lambda-layer` a. Users using new layers have to manually choose the version, so there's no risk of someone upgrading the version without manually doing it. See https://docs.sentry.io/platforms/node/guides/aws-lambda/layer/ b. It's easy to undo: logging in with a write-permission account and deleting the latest layer. c. Users aren't notified. d. The versions the AWS Lambda layer integration show are picked from the registry. e. Low impact, easy to revert. 2. `gcs` a. Only affects CDN, and users have to manually upgrade the version. See https://docs.sentry.io/platforms/javascript/install/cdn/ b. Users aren't notified. c. Can revert it by removing the files from GCS. d. Low impact, easy to revert. 3. `github` a. It may notify users. b. Creates a release, a tag, and links them on GitHub. To undo, can manually remove everything created from the GitHub UI. c. Medium impact, easy to revert. 4. `npm` a. Affects users installing the SDKs for the first time, and to those upgrading them. b. High impact, can't revert. 5. `registry` a. Sentry notifies customers based on updates to the registry. We don't want to notify them if a release doesn't go well. b. We own it and can revert the changes with ease, but the prior point is bad enough. c. Very high impact, can revert.
size-limit report
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Do we need to change this in other SDKs as well?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Verify good explanation of the motivation, thanks for writing it down.
github
...
b. Creates a release, a tag, and links them on GitHub. To undo, can manually
remove everything created from the GitHub UI.
While it is technically possible to do, this can also have bad consequences. It is bad practice to delete or otherwise change published tags, and could break downstream systems (like caches/mirrors, forks/clones, CIs, ...).
Once published, we should never remove a tag/release from GitHub lightly.
Or the alternative -- make Craft run targets in a certain order regardless of the YAML config. Pros: no need to change every repo's |
The order of targets Craft runs in a release matters. If a target fails, Craft exits and stops the release, not running the targets that come after the failed one. For this reason, targets that are easy to undo and don't have big consequences should come first, and those that are harder to undo and have more impact should run last; i.e. targets are sorted by ascending impact/cost of reverting ratio. Based on that guideline, this is the new order: 1. `aws-lambda-layer` a. Users using new layers have to manually choose the version, so there's no risk of someone upgrading the version without manually doing it. See https://docs.sentry.io/platforms/node/guides/aws-lambda/layer/ b. It's easy to undo: logging in with a write-permission account and deleting the latest layer. c. Users aren't notified. d. The versions the AWS Lambda layer integration show are picked from the registry. e. Low impact, easy to revert. 2. `gcs` a. Only affects CDN, and users have to manually upgrade the version. See https://docs.sentry.io/platforms/javascript/install/cdn/ b. Users aren't notified. c. Can revert it by removing the files from GCS. d. Low impact, easy to revert. 3. `github` a. It may notify users. b. Creates a release, a tag, and links them on GitHub. To undo, can manually remove everything created from the GitHub UI. c. Medium impact, easy to revert. 4. `npm` a. Affects users installing the SDKs for the first time, and to those upgrading them. b. High impact, can't revert after 72h. 5. `registry` a. Sentry notifies customers based on updates to the registry. We don't want to notify them if a release doesn't go well. b. We own it and can revert the changes with ease, but the prior point is bad enough. c. Very high impact, can revert.
The goal of this PR is to minimize harm when a release fails. For example, last
week this release failed, and
with the changes this PR makes, the
npm
target wouldn't have run.The order of targets Craft runs in a release matters. If a target fails, Craft
exits and stops the release, not running the targets that come after the failed
one. For this reason, targets that are easy to undo and don't have big
consequences should come first, and those that are harder to undo and have more
impact should run last; i.e. targets are sorted by ascending impact/cost of
reverting ratio.
Based on that guideline, this is the new order:
aws-lambda-layer
a. Users using new layers have to manually choose the version, so there's no
risk of someone upgrading the version without manually doing it. See
integration docs.
b. It's easy to undo: logging in with a write-permission account and deleting
the latest layer.
c. Users aren't notified.
d. The versions the AWS Lambda layer integration show are picked from the
registry.
e. Low impact, easy to revert.
gcs
a. Only affects CDN, and users have to manually upgrade the version. See
CDN docs.
b. Users aren't notified.
c. Can revert it by removing the files from GCS.
d. Low impact, easy to revert.
github
a. It may notify users.
b. Creates a release, a tag, and links them on GitHub. To undo, can manually
remove everything created from the GitHub UI.
c. Medium impact, easy to revert.
npm
a. Affects users installing the SDKs for the first time, and to those upgrading
them.
b. High impact, can't revert after 72h.
registry
a. Sentry notifies customers based on updates to the registry. We don't want
to notify them if a release doesn't go well.
b. We own it and can revert the changes with ease, but the prior point is bad
enough.
c. Very high impact, can revert.