Skip to content
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

chore(exceptions): Fix up what exceptions are thrown #3835

Merged
merged 2 commits into from
Jul 26, 2020

Conversation

marchello2000
Copy link
Contributor

In an effor to clean up logs to improve signal to noise ratio of bug vs non-essential/expected failures:

  • clean up what exceptions are thrown from a bunch of places
  • anything deriving from UserException won't get logged as a failure
  • for now, change capacity short circuit to warn instead of error because it is mostly noise. I will work on making it provide accurate signal separately

In an effor to clean up logs to improve signal to noise ratio of bug vs non-essential/expected failures:
* clean up what exceptions are thrown from a bunch of places
* anything deriving from UserException won't get logged as a failure
* for now, change capacity short circuit to warn instead of error because it is mostly noise. I will work on making it provide accurate signal separately
@@ -82,7 +84,7 @@ class ClusterSizePreconditionTask extends AbstractCloudProviderAwareTask impleme
errors << 'Missing regions'
}
if (errors) {
throw new IllegalStateException("Invalid configuration " + errors.join(','))
throw new ConfigurationException("Invalid configuration " + errors.join(','))
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
throw new ConfigurationException("Invalid configuration " + errors.join(','))
throw new ConfigurationException("Invalid configuration " + errors.join(', '))

just to be consistent with line 136?

this.errors = errors;
}

public PipelineTemplateValidationException(
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Do we need all these constructors?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

silly copy-paste, thank you! removing a bunch of them

import java.util.Collections;
import java.util.Map;

public class PipelineTemplateValidationException extends UserException
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
public class PipelineTemplateValidationException extends UserException
public final class PipelineTemplateValidationException extends UserException

@marchello2000 marchello2000 added the ready to merge Approved and ready for merge label Jul 26, 2020
@mergify mergify bot merged commit c47e530 into spinnaker:master Jul 26, 2020
@mergify mergify bot added the auto merged Merged automatically by a bot label Jul 26, 2020
@marchello2000 marchello2000 deleted the mark/exceptions branch July 26, 2020 20:40
marchello2000 added a commit to marchello2000/orca that referenced this pull request Jul 28, 2020
I made changes in my previous PR (spinnaker#3835) to throw a new exception, but didn't update the catch so this is not again generating noise
mergify bot added a commit that referenced this pull request Jul 28, 2020
I made changes in my previous PR (#3835) to throw a new exception, but didn't update the catch so this is not again generating noise

Co-authored-by: mergify[bot] <37929162+mergify[bot]@users.noreply.github.com>
KathrynLewis pushed a commit to KathrynLewis/orca that referenced this pull request Jan 31, 2021
* chore(exceptions): Fix up what exceptions are thrown

In an effor to clean up logs to improve signal to noise ratio of bug vs non-essential/expected failures:
* clean up what exceptions are thrown from a bunch of places
* anything deriving from UserException won't get logged as a failure
* for now, change capacity short circuit to warn instead of error because it is mostly noise. I will work on making it provide accurate signal separately

* fixup! chore(exceptions): Fix up what exceptions are thrown
KathrynLewis pushed a commit to KathrynLewis/orca that referenced this pull request Jan 31, 2021
I made changes in my previous PR (spinnaker#3835) to throw a new exception, but didn't update the catch so this is not again generating noise

Co-authored-by: mergify[bot] <37929162+mergify[bot]@users.noreply.github.com>
jervi added a commit to jervi/orca that referenced this pull request Feb 8, 2021
Template validation errors are supposed to bubble up to the surface, but this broke in spinnaker#3835. This PR brings them back.
jervi added a commit to jervi/orca that referenced this pull request Mar 15, 2021
Template validation errors are supposed to bubble up to the surface, but this broke in spinnaker#3835. This PR brings them back.
mergify bot added a commit that referenced this pull request Dec 4, 2021
* fix(pipeline): Bring back template validation messages in Deck

Template validation errors are supposed to bubble up to the surface, but this broke in #3835. This PR brings them back.

* Trigger new build

Co-authored-by: mergify[bot] <37929162+mergify[bot]@users.noreply.github.com>
Co-authored-by: Chris Thielen <christopherthielen@users.noreply.github.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
auto merged Merged automatically by a bot ready to merge Approved and ready for merge
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants