-
Notifications
You must be signed in to change notification settings - Fork 251
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
Add write operation to change toggle state via Spring Boot Actuator #310
Conversation
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.
That looks great! I added some reviews for minor improvements.
@@ -36,9 +39,14 @@ | |||
@Endpoint(id = "togglz") | |||
public class TogglzEndpoint { | |||
|
|||
private final FeatureProvider featureProvider; | |||
|
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.
I don't think we need FeatureProvider
. You can get the list of features with featureManager.getFeatures()
, like in getAllFeatures
methods.
|
||
@WriteOperation | ||
public TogglzFeature setFeatureState(@Selector String featureName, boolean enabled) { | ||
final Feature feature = featureProvider.getFeatures().stream() |
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.
We should use featureManager.getFeatures()
here instead.
return new TogglzFeature(feature, featureState); | ||
} | ||
|
||
return null; |
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.
I would prefer if we still returned a TogglzFeature
even if the state didn't change.
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.
I thought I'd be nice to return null
here because in that case the actuator will respond with HTTP 204 instead of HTTP 200 which enables the client to deal with no actual changes (if someone would need that).
If you still prefer to return something else let's discuss that. In order to provide a more advanced HTTP response we need to add a @EndpointWebExtension
I think.
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.
Yes, I understood your idea, but I still think we should return a richer object. With @EndpointWebExtension
that seems to be possible, so maybe we should go that way.
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.
OK, now the endpoint always performs the set feature state operation and returns its result.
Resolves: #308