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

Javadoc of ReactorResourceFactory#setConnectionProviderSupplier wrongly states it can be ignored #33338

Closed
whojes-toss opened this issue Aug 7, 2024 · 4 comments
Assignees
Labels
in: web Issues in web modules (web, webmvc, webflux, websocket) type: documentation A documentation task
Milestone

Comments

@whojes-toss
Copy link

/**
* Use this when you don't want to participate in global resources and
* you want to customize the creation of the managed {@code ConnectionProvider}.
* <p>By default, {@code ConnectionProvider.elastic("http")} is used.
* <p>Note that this option is ignored if {@code userGlobalResources=false} or
* {@link #setConnectionProvider(ConnectionProvider)} is set.
* @param supplier the supplier to use
*/
public void setConnectionProviderSupplier(Supplier<ConnectionProvider> supplier) {
this.connectionProviderSupplier = supplier;

I wonder whether useGlobalResources=false option ignores connectionProviderSupplieroption or not cause the code seems it's not

public void start() {
synchronized (this.lifecycleMonitor) {
if (!this.running) {
if (this.useGlobalResources) {
Assert.isTrue(this.loopResources == null && this.connectionProvider == null,
"'useGlobalResources' is mutually exclusive with explicitly configured resources");
HttpResources httpResources = HttpResources.get();
if (this.globalResourcesConsumer != null) {
this.globalResourcesConsumer.accept(httpResources);
}
this.connectionProvider = httpResources;
this.loopResources = httpResources;
}
else {
if (this.loopResources == null) {
this.manageLoopResources = true;
this.loopResources = this.loopResourcesSupplier.get();
}
if (this.connectionProvider == null) {
this.manageConnectionProvider = true;
this.connectionProvider = this.connectionProviderSupplier.get();
}
}
this.running = true;
}
}
}

if i'm wrong enlighten me please
thank you in advance

@spring-projects-issues spring-projects-issues added the status: waiting-for-triage An issue we've not yet triaged or decided on label Aug 7, 2024
@snicoll
Copy link
Member

snicoll commented Aug 7, 2024

It's making sure that connectionProvider is null, then it assigns it. If it is assigned the supplier is never invoked.

@snicoll snicoll closed this as not planned Won't fix, can't repro, duplicate, stale Aug 7, 2024
@snicoll snicoll added status: invalid An issue that we don't feel is valid and removed status: waiting-for-triage An issue we've not yet triaged or decided on labels Aug 7, 2024
@whojes-toss
Copy link
Author

yeah but i was pointing out that the comment says Note that this option is ignored if {@code userGlobalResources=false} and it is wrong because when userGlobalResources=false && connectionProvider==null the supplier will be invoked.

@snicoll snicoll reopened this Aug 8, 2024
@snicoll snicoll added status: waiting-for-triage An issue we've not yet triaged or decided on and removed status: invalid An issue that we don't feel is valid labels Aug 8, 2024
@snicoll
Copy link
Member

snicoll commented Aug 8, 2024

Thanks for following-up. Looks like I looked at the wrong branch. We'll have another look.

@jhoeller jhoeller added the in: web Issues in web modules (web, webmvc, webflux, websocket) label Aug 8, 2024
@snicoll snicoll changed the title Does ReactorResourceFactory have wrong comments? Javadoc of ReactorResourceFactory#setConnectionProviderSupplier wrongly states it can be ignored Aug 8, 2024
@simonbasle simonbasle self-assigned this Aug 8, 2024
@simonbasle simonbasle modified the milestones: 6.1.x, 6.1.12 Aug 8, 2024
@simonbasle simonbasle added type: documentation A documentation task and removed status: waiting-for-triage An issue we've not yet triaged or decided on labels Aug 8, 2024
@simonbasle
Copy link
Contributor

The phrasing is indeed misleading, and similarly in #setLoopResources(Supplier<LoopResources>)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
in: web Issues in web modules (web, webmvc, webflux, websocket) type: documentation A documentation task
Projects
None yet
Development

No branches or pull requests

5 participants