feat(publish): Store and restore publish state#192
Conversation
A very common occurance is publishing being inturrupted in the middle by some error, causing the whole publish operation to stop and the operator to resume by explicitly determining the unpublished targets and then passing these via the `-t` argument. This patch adds a simple JSON state file, keyed by the release version that gets deleted after a successful publish. When the file exists, it is automatically used to skip over already published targets.
|
I think it is time to rethink how we handle |
|
|
||
| /** Options for "cocoapods" target */ | ||
| export interface CocoapodsTargetOptions extends TargetConfig { | ||
| export interface CocoapodsTargetOptions { |
There was a problem hiding this comment.
What's the main motivation behind this?
There was a problem hiding this comment.
No motivation, just pleasing TypeScript. Once I made the name field required, all these configs started to give type errors, complaining about a missing name attribute. Upon inspection, I realized none of these were actually extending TargetConfig, there was nothing shared. These were internal configs. What needed to be the TargetConfig is the one we pass in the constructor, which I refactored already.
chadwhitacre
left a comment
There was a problem hiding this comment.
Does this actually help us, though, in the context of a GitHub Actions deploy? How will the state file persist from one run to the next?
| if (cleanupEnabled) { | ||
| // We intentionally do not block on the clean up operation | ||
| // so wait ~100ms before checking | ||
| await new Promise(resolve => setTimeout(resolve, 100)); |
There was a problem hiding this comment.
Sleep is never a good idea. I mean, sleep is a great idea ... but not here. No better way? Is this related to the await removals that @tonyo is asking about?
There was a problem hiding this comment.
Sleep is never a good idea. I mean, sleep is a great idea ... but not here.
Totally agree.
No better way?
We can try polling with a... timeout?
Is this related to the await removals that @tonyo is asking about?
Yup :(
Not yet. We'll need to hook this up into our fail handler and then store/serialize this data to be unstored/unserialized at the start of the job. |
Builds on getsentry/craft#192 to limit targets to publish to. In a follow up, we'll add updating this list automatically on failure for idempotent retries on this repo.
Builds on getsentry/craft#192 to limit targets to publish to. In a follow up, we'll add updating this list automatically on failure for idempotent retries on this repo.
This is a follow up to #192. It fixes a bug that occurs when a publish state file does not list all possible targets. In this case any unlisted target, is skipped whereas its state should be treated as "unpublished". This stemmed from a logic error in the if statement which skipped publishing when the special target type "none" was passed. The PR also refactors the code around this a bit to make it easier to follow and less prone to errors. Ideal follow ups would be more tests around publish and the state file functionality.
This is a follow up to #192. It fixes a bug that occurs when a publish state file does not list all possible targets. In this case any unlisted target, is skipped whereas its state should be treated as "unpublished". This stemmed from a logic error in the if statement which skipped publishing when the special target type "none" was passed. The PR also refactors the code around this a bit to make it easier to follow and less prone to errors. Ideal follow ups would be more tests around publish and the state file functionality.
A very common occurrence is publishing being interrupted in the
middle by some error, causing the whole publish operation to stop
and the operator to resume by explicitly determining the unpublished
targets and then passing these via the
-targument.This patch adds a simple JSON state file, keyed by the release version
that gets deleted after a successful publish. When the file exists, it is
automatically used to skip over already published targets.
Fixes #179. Also refactors the publish command a bit to make things work
better with this new system.