WIP: publish files for discussion #24

reimic · 2024-02-22T12:52:41Z

🚧 Work in progress 🚧

uploaded for ease of discussion only, even if parts of this will become the basis for changes to the codebase it will be done in a separate PR

Issues to discuss:

runner validation policy, precision of exceptions, (kinda blocks adding tests !)
role of resource / file / context / runtime managers,
testing and documentation.

reimic · 2024-02-22T13:24:40Z

runner validation policy

After the JSON was properly validated and parsed, the resource needs have been collected, the steps are run. So:

We have been discussing this on slack for a while now, pr #18 reflected that with a more offensive approach to exceptions. But while approaching other steps I begun to wonder:

to what extent would you want things offensively breaking execution?
how precise should the feedback be?
how should the continue on error option actually benefit the end-user?
a. not to cause a cascade of issues and obfuscate the root cause?
b. which steps would actually benefit from this feature?
c. this implies blueprints are a sort of execution tree - would there be a benefit to declaring it overtly?
for a failing blueprint:
a. what is the expected state?
b. should all changes be reverted?
c. if not should all procured resources be discarded?
d. if not - how should steps know a step was run and did not fail?

As an idea:
I was wondering about the utility a blueprint running mode that fails fast when a particular step fails but instead of failing the whole blueprint, returns control to the user for troubleshooting (this could prove useful for blueprint designers), and after corrections - continues from the last point of failure. To get a mental image think - blueprint wizard. This could yield a running installation and a final, successful blueprint.

adamziel · 2024-02-23T09:29:59Z

src/WordPress/Blueprints/Runner/Step/DownloadWordPressStepRunner.php

@@ -7,6 +7,7 @@

 class DownloadWordPressStepRunner extends InstallAssetStepRunner {

+    //TODO why is this download and not install?


Installation is a separate step. The developer may not want to run the installation wizard automatically.

adamziel · 2024-02-23T09:30:57Z

src/WordPress/Blueprints/Runner/Step/InstallAssetStepRunner.php

@@ -10,6 +10,8 @@


 class InstallAssetStepRunner extends BaseStepRunner {
+// TODO following the github API - this is not an exposed step - maybe it's internals should be used by other steps and not extended?


I had the same thought, just couldn't think of the right names for things.

adamziel · 2024-02-23T09:32:10Z

src/WordPress/Blueprints/Runner/Step/MkdirStepRunner.php

-		}
+    function run(MkdirStep $input) {
+        $path = $input->path;
+        $this->fileManager->assertNoFileExists($path);


This line obscures that $path is relative to the document root and needs to be resolved.

adamziel · 2024-02-23T09:44:05Z

to what extent would you want things offensively breaking execution?

As a rule of thumb, steps are atomic commands that should always fail when they can't accomplish the task. It's up to the runner to decide what to do with that failure.

how precise should the feedback be?

Let's start with just enough feedback for the developer to debug their issue. Then we'll improve the error messages in any place that will confuse the API consumers.

how should the continue on error option actually benefit the end-user?

Here's the entry to the rabbit hole:

WordPress/wordpress-playground#605

tl;dr – some failures may not be worth crashing the entire Blueprint execution pipeline. I may have an optional plugin or a content export file and I'm fine if they can't be set up.

a. not to cause a cascade of issues and obfuscate the root cause?

The runner could still produce warnings when a non-critical step fails.

b. which steps would actually benefit from this feature?

It's up to the developers to decide, they would explicitly mark specific steps in their Blueprints as "okay to fail".

c. this implies blueprints are a sort of execution tree - would there be a benefit to declaring it overtly?

What do you mean by a tree? I think of it as a linear flow like a Dockerfile.

for a failing blueprint:
a. what is the expected state?

For now, any state that was left on the disk after the last successful step.

b. should all changes be reverted?

Not in v1.

c. if not should all procured resources be discarded?

What do you mean? I don't understand this question.

d. if not - how should steps know a step was run and not fail?

The steps shouldn't be concerned with that at all. A step only has an input and an output, it doesn't care about the resolution logic or execution order.

As an idea -I was wondering about the utility a blueprint running mode that fails fast when a particular step fails but instead of failing the whole blueprint, returns control to the user for troubleshooting (this could prove useful for blueprint designers), and after corrections - continues from the last point of failure. To get a mental image think - blueprint wizard. This could yield a running installation and a final, successfull blueprint.

It will take some time to get there, but it would be a great developer tool!

reimic · 2024-02-23T11:19:30Z

Ok. Thanks for the details answers and links. This allows me to continue adding tests. Huzzah! I now see the heuristic I should take.

b. should all changes be reverted?

Not in v1.

I see this an a issue that should be addressed rather quickly. Since, this means after a blueprint fails somewhere in the middle. It can't be rerun easily. I'd imagine achieving effective idempotency could be a goal here.

c. if not should all procured resources be discarded?

What do you mean? I don't understand this question.

I run a blueprint once. It gets me a bunch of resources. But fails at the very last moment. I run it the second time and the resources that were procured are already there. If not, we get the worst of both world - on one hand, the changes of steps are not reverted, and on the other - the resource streams have to be prepared anew.

adamziel · 2024-02-26T11:34:13Z

I see this an a issue that should be addressed rather quickly. Since, this means after a blueprint fails somewhere in the middle. It can't be rerun easily. I'd imagine achieving effective idempotency could be a goal here.

+1

I run it the second time and the resources that were procured are already ther

Aha! The runtime will be responsible for caching the downloads. Network transfer should only happen on cache miss.

publish files for discussion

8f391aa

adamziel reviewed Feb 23, 2024

View reviewed changes

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

WIP: publish files for discussion #24

WIP: publish files for discussion #24

reimic commented Feb 22, 2024 •

edited

Loading

reimic commented Feb 22, 2024 •

edited

Loading

adamziel Feb 23, 2024 •

edited

Loading

adamziel Feb 23, 2024

adamziel Feb 23, 2024

adamziel commented Feb 23, 2024

reimic commented Feb 23, 2024

adamziel commented Feb 26, 2024

		@@ -7,6 +7,7 @@

		class DownloadWordPressStepRunner extends InstallAssetStepRunner {

		//TODO why is this download and not install?

		@@ -10,6 +10,8 @@


		class InstallAssetStepRunner extends BaseStepRunner {
		// TODO following the github API - this is not an exposed step - maybe it's internals should be used by other steps and not extended?

WIP: publish files for discussion #24

Are you sure you want to change the base?

WIP: publish files for discussion #24

Conversation

reimic commented Feb 22, 2024 • edited Loading

reimic commented Feb 22, 2024 • edited Loading

adamziel Feb 23, 2024 • edited Loading

Choose a reason for hiding this comment

adamziel Feb 23, 2024

Choose a reason for hiding this comment

adamziel Feb 23, 2024

Choose a reason for hiding this comment

adamziel commented Feb 23, 2024

reimic commented Feb 23, 2024

adamziel commented Feb 26, 2024

reimic commented Feb 22, 2024 •

edited

Loading

reimic commented Feb 22, 2024 •

edited

Loading

adamziel Feb 23, 2024 •

edited

Loading