-
Notifications
You must be signed in to change notification settings - Fork 38
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
(feat) Adding Chaos Engineering #136
Conversation
Hey @natenho, mind taking a look at this PR please? |
{ | ||
httpResponse.StatusCode = (int)HttpStatusCode.InternalServerError; | ||
|
||
var bodyBytes = Encoding.UTF8.GetBytes($"Error {httpResponse.StatusCode}: {HttpStatusCode.ServiceUnavailable}"); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The status code in the string is ServiceUnavailable, I suppose you mean InternalServerError, right?
|
||
private static IServiceCollection AddChaosServices(this IServiceCollection services) => | ||
services | ||
.AddSingleton<IChaosStrategy, ChaosStrategyBehavior>() |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Do we really need singletons here? I would make any dependency as transient by default if I dont need a singleton..it is much easier to deal with concurrent requests in transient objects if we need some context inside the behavior
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@lanegoolsby there you go! The code is very good, just some minor issues, we can merge it asap if you could please review the pr comments! :)
@lanegoolsby I took the liberty of making the changes to avoid taking even more time to do the merge. Thank you for your contribution! |
Thank you!! |
This is a direct copy of #114, I just addressed the PR feedback. I have a need for this feature and that other PR stalled. :)