Kong / unirest-java

Unirest in Java: Simplified, lightweight HTTP client library.
http://kong.github.io/unirest-java/
MIT License
2.6k stars 592 forks source link

Multiple configuration instances clarification #318

Closed cdx-grillo closed 4 years ago

cdx-grillo commented 4 years ago

From the docs ... UnirestInstance unirest = Unirest.primaryInstance(); unirest.config().connectTimeout(5000); Performing this action will change the connectTimeout for all subsequent calls to Unirest.get(".."), correct?

So, if we really need a different Unirest configuration, say I wanted to change followRedirects for one specific request, I would need to use UnirestInstance unirest = Unirest.spawnInstance(); unirest.config().followRedirects(false) and when done with this request, call unirest.shutdown()

I was just slightly confused when reading about the multiple configuration instance support and the example with the Unirest.primaryInstance() appears to modify the underlying config for all subsequent Unirest.* calls.

ryber commented 4 years ago

Generally speaking, if it's something that can be done on a per-request basis, then it's going to be present in the request builder. Those things in the config are not things that should be done repeatedly. The reason for this is that the engine that runs Unirest (Apache Http Client), is pretty heavy and starting it spwans threads, so if you were to attempt to create a new config on every request you would probably have some problems. In any event I don't think the performance would be great.

There are essentially two different config touch points for the apache engine, the clients themselves (one "regular", one async), and a per-request config. They share some, but not all options. It looks like followredirect for example is in both, but Unirest only exposes it on the main config and not on the request. If you find options like this that you would like to tweek per request we can pull those down and override the client with the request (IDK if that was one you actually wanted to override or you were just using it as an example)

cdx-grillo commented 4 years ago

Thanks very much for the detailed response. My actual use case is with the Unirest.config().verifySsl(false); where a user enters an https url they know is using a self-signed cert so we give them an option to ignore ssl validation via the UI.

I think I can just have 2 separate Unirest services with different configurations; one for the standard config and a separate one that uses Unirest.spawnInstance() that turns off ssl validation. I would create both of these once on app init and then just call one or the other based on the user input. I would make sure to manually add the shutdown hook for the spawned instance spawnInstance.config().addShutdownHook(true). This seems like a reasonable solution when the app needs to support multiple Unirest configurations without having to create new Unirest instances for each request.

ryber commented 4 years ago

yeah, the SSL stuff is very heavy and built into the client. The Apache client is pretty hostile to the idea of turning off certs. I'm about to release a new version that lets you overwrite the SSLContext which might give you some control to do it via routes and say "https://dev-server" has it off but "https://prod-server" has it on, but also, SSLContexts are really complicated and hard to mess with.

cdx-grillo commented 4 years ago

That sounds like a good feature add, I could definitely make use of that in other web apps we have using Unirest. I will close the issue if there are no other discussion points.

theyuv commented 4 years ago

@ryber Is changing the number of retry attempts (or the fact that Unirest retries at all) something that can be done on a "per-request basis"? Or is this something that I should implement using multiple Unirest instances?

ryber commented 4 years ago

It's not currently per-request, nor does it look like it could be as the dependent Apache implementation doesn't support that. You shouldn't use a different Unirest instance per request either. It's not designed for that