Postponed (maintainer needs more info)
Project:
Automatic Updates
Version:
3.0.x-dev
Component:
Package Manager
Priority:
Major
Category:
Task
Assigned:
Issue tags:
Reporter:
Created:
3 May 2023 at 14:05 UTC
Updated:
16 May 2023 at 02:04 UTC
Jump to comment: Most recent, Most recent file
, or via debugger:
Comments
Comment #3
wim leersTricky:
sleep()does not count against execution time! 🤯Comment #4
wim leersComment #5
wim leersComment #6
wim leersThe commit I just pushed works in manual testing, but the automated test is very difficult to write. Will probably only finish that tomorrow.
Comment #7
wim leersPushed what I got so far that AFAICT proves writing a
Kerneltest for this is impossible.Maybe I can write a functional test for this, but I've not been very successful so far 🫠
Marking for the first commit.
Comment #8
tedbowI need to review
Comment #9
tedbowI don't think we want to use Failure marker for this. The class doc states
So this is for a very particular purpose, "commit operation failed midway through, which could leave the site
* code base in an indeterminate state". I created #3360548: Document what the existence of the PACKAGE_MANAGER_FAILURE.yml means to make this clearer
I don't we should reuse it for this. We stop all operations if this file exists. The purpose of this file is that you can't trust your site's code at that point.
Of the 5 places we use this only during Apply would it leave your site's code base in an indeterminate state. But since it added to the first line of
\Drupal\package_manager\StageBase::applyfor all we know the time out is from very badly written PreApplyEvent subscriber. In that case we should not write the marker file. InStageBase::applyif call$this->committer->commit($stage_dir, $active_dir, $paths_to_exclude, NULL, $timeout);and we never return we will have already written the marker file and it will not be delete if the execution does not return toStageBase::apply. In that case still think we want to totally replace the file though inlogExecutionTimeFailureas this MR does because it currently doesn't mention anything about restoring your site from a backup like\Drupal\package_manager\StageBase::getFailureMarkerMessage().I will make some more specific suggestions in the MR
Comment #10
tedbow@Wim Leers can you update the summary to state concisely what the problem is we are trying to address. The Problem/Motivation section currently just links to another issue comment and that issue comment then mentions the 1 above it and then links to yet issue
Comment #11
tedbowPostponing this because re #10 I don't have the whole context of the problem space we are trying to solve
Comment #12
tedbow