Defer projection failure until after plugins run #1416
Conversation
|
|
This changes `SmithyBuildImpl` to continue applying plugins after one fails, throwing the error (if present) after they have all completed. This is useful for example when you want to see the serialized output of a model you know is valid but a plugin is causing the whole build to fail. A new test case was added to ensure that artifacts produced by valid plugins are still created despite the build failing.
milesziemer
force-pushed
the
defer-plugin-failure
branch
from
6f75f38 to
1fc7385
Compare
milesziemer
changed the title
| @@ -283,13 +283,24 @@ private ProjectionResult applyProjection( | |||
| .model(projectedModel) | |||
| .events(modelResult.getValidationEvents()); | |||
|
|
|||
| // Keep track of the first error created by plugins to fail the build after all plugins have run | |||
| Throwable firstPluginError = null; | |||
There was a problem hiding this comment.
Will this only keep track of a single plugin error? This might be confusing if multiple plugins fail, but only the error for the first (or last?) ends up being thrown at the end.
There was a problem hiding this comment.
This only keeps track of the first plugin error. My initial thinking was to keep track of all plugin errors, but errors are handled per-projection (in SmithyBuild and BuildCommand). In order to not change these, we would need to aggregate all plugin errors into one error to throw as the projection error, but I wanted to defer making a decision on what exactly this would look like. Every plugin error is logged however, see here, but I think it would be nice to throw one big exception with all plugin errors
There was a problem hiding this comment.
That sounds like a bigger change that could be done later, but this still seems like a good improvement for now.
…thy-lang#1416)" (smithy-lang#1429)" This reverts commit 18676ec.
This changes `SmithyBuildImpl` to continue applying plugins after one fails, throwing the error (if present) after they have all completed. This is useful for example when you want to see the serialized output of a model you know is valid but a plugin is causing the whole build to fail. A new test case was added to ensure that artifacts produced by valid plugins are still created despite the build failing. This was originally implemented in smithy-lang#1416, but was rolled back in smithy-lang#1429 as a precaution since we had an unrelated issue ocurring at the time.
This changes `SmithyBuildImpl` to continue applying plugins after one fails, throwing the error (if present) after they have all completed. This is useful for example when you want to see the serialized output of a model you know is valid but a plugin is causing the whole build to fail. A new test case was added to ensure that artifacts produced by valid plugins are still created despite the build failing. This was originally implemented in #1416, but was rolled back in #1429 as a precaution since we had an unrelated issue ocurring at the time.
This changes `SmithyBuildImpl` to continue applying plugins after one fails, throwing the error (if present) after they have all completed. This is useful for example when you want to see the serialized output of a model you know is valid but a plugin is causing the whole build to fail. A new test case was added to ensure that artifacts produced by valid plugins are still created despite the build failing. This was originally implemented in smithy-lang#1416, but was rolled back in smithy-lang#1429 as a precaution since we had an unrelated issue ocurring at the time.
Issue: #1277
This changes
SmithyBuildImplto continue applying plugins after one fails,throwing the error (if present) after they have all completed. This is
useful for example when you want to see the serialized output of a model
you know is valid but a plugin is causing the whole build to fail. A new
test case was added to ensure that artifacts produced by valid plugins
are still created despite the build failing.
By submitting this pull request, I confirm that my contribution is made under the terms of the Apache 2.0 license.