Skip to content

Review @ConditionalOnExposedEndpoint #16169

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

Closed
bclozel opened this issue Mar 8, 2019 · 3 comments
Closed

Review @ConditionalOnExposedEndpoint #16169

bclozel opened this issue Mar 8, 2019 · 3 comments
Assignees
Labels
type: task A general task
Milestone

Comments

@bclozel
Copy link
Member

bclozel commented Mar 8, 2019

Since #16093, a new conditional annotation has been introduced to guard Actuator endpoints and only instantiate infrastructure when endpoints are exposed.

Given this comment on the issue:

The current implementation is completely separating enablement and exposure.
For now, @ConditionalOnEnabledEndpoint and @ConditionalOnExposedEndpoint are used together. It might make sense to revisit that in the next milestone and make @ConditionalOnExposedEndpoint call the other. Right now I'm wondering if it's still best to keep those separated as they are different concepts and there might be cases where we want to let developers reuse/extend existing infrastructure independently of what's happening with the endpoints.

We should review this annotation and consider whether we want to merge condition implementations, or consider both conditions when @ConditionalOnExposedEndpoint is added.

@bclozel bclozel added the type: task A general task label Mar 8, 2019
@bclozel bclozel added this to the 2.2.x milestone Mar 8, 2019
@snicoll
Copy link
Member

snicoll commented Apr 28, 2019

How about ConditionalOnAvailableEndpoint? Or perhaps we only need that and not the exposed one? I wonder if there is a case to check if it's exposed but not enabled. If there isn't we could replace ConditionalOnExposedEndpoint by @ConditionalOnAvailableEndpoint that checks both.

That way, we'd have a single annotation for both purposes.

@snicoll snicoll modified the milestones: 2.2.x, 2.2.0.M3 Apr 28, 2019
@snicoll snicoll modified the milestones: 2.2.0.M3, 2.2.x May 13, 2019
@snicoll snicoll added the for: team-attention An issue we'd like other members of the team to review label May 13, 2019
@bclozel bclozel modified the milestones: 2.2.x, 2.2.0.M4 May 13, 2019
@bclozel
Copy link
Member Author

bclozel commented May 13, 2019

My vote goes to deprecating @ConditionalOnEnabledEndpoint and @ConditionalOnExposedEndpoint, and merging them both into a new @ConditionalOnAvailableEndpoint.

@snicoll snicoll self-assigned this May 20, 2019
@snicoll snicoll removed the for: team-attention An issue we'd like other members of the team to review label May 20, 2019
@snicoll
Copy link
Member

snicoll commented May 20, 2019

@ConditionalOnEnableEndpoint is now deprecated and users are invited to move to @ConditionalOnAvailableEndpoint to get extra checks that prevent the bean to be created if the underlying endpoint is not going to be used in practice.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
type: task A general task
Projects
None yet
Development

No branches or pull requests

2 participants