-
Notifications
You must be signed in to change notification settings - Fork 100
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
Support Jetty client version > 9.x #181
Comments
There is currently no way to do this, however we are working on a "bring your own http client" feature that we hope to release later this year that should solve this for you. Stay tuned. |
We just ran into this because we've switched from Jetty 9 to Jetty 11 across the board, due to 9.x being out of community support, only to find that the Cognitect AWS client breaks. |
Update: aws-api depends on the cognitect http-client, and is subject to its jetty dependency. Since that http-client has other users besides aws-api, we can't depend on it upgrading its jetty dependency on any timeline, if ever. We have done some design work on the "bring your own http client" feature, and while I can't really give you a timeline, I can tell you the direction:
Given that plan, you could do the following right now, with the understanding that you are using undocumented APIs/features that are very likely to change before we publish them. You've been warned! WARNING: 🚧 THE FOLLOWING IS NOT OFFICIAL AND COMES WITH ZERO PROMISES 🚧
(cognitect.aws.client.api/client {:api <api> :http-client (make-my-client)}) For now, you'll have do that for every aws-client you create, but we plan to make it so you can register a default when we release this feature. WARNING: 🚧 THE PRECEDING IS NOT OFFICIAL AND COMES WITH ZERO PROMISES 🚧 If anybody tries this, please report back on your success! Once we release this, we'll be looking for others in the community to release wrappers for e.g. jetty >= 10 and any other http clients you wish to use. |
FWIW, I replaced the cognitect http-client with a JDK 11 java.net.http version for awyeah-api: https://github.com/grzm/awyeah-api/blob/main/src/com/grzm/awyeah/http_client.clj Currently it has one known infelicity: I'm sure there are others. It's purpose was to mimic as faithfully as possible the behavior of the cognitect http-client to limit surprises. It wouldn't surprise me if this isn't the best way to construct such a client. |
@grzm s/congitect.http-client/cognitect.http-client ;) Also, thanks for posting! |
LOL. The number of times I've made that typo is embarrassing :) |
Don't tell anybody, but I'll bet I've mistyped chelimsky more than you've mistyped cognitect ;) |
Here's what I had to do in order to use an HTTP proxy successfully. Start by looking at the source code for (defn create
"Creates an http-client that can be used with submit. Takes a config map with
the following keys:
:resolve-timeout in msec, default 5000
:connect-timeout in msec, default 5000
:max-connections-per-destination default 64
:pending-ops-limit default 64
:classpath-trust-store classpath location of trust store
:trust-store-password trust store password
:trust-store java.security.KeyStore instance
:validate-hostnames boolean, defaults to true
:proxy-host optional host to use for proxy, string, defaults to 'localhost' if proxy-port is provided with no proxy-host
:proxy-port optional port to use for proxy, int"
[{:keys [resolve-timeout connect-timeout max-connections-per-destination
pending-ops-limit proxy-host proxy-port]
:or {resolve-timeout 5000
connect-timeout 5000
max-connections-per-destination 64
pending-ops-limit 64}
:as config}]
(configure-jetty-announce)
(let [jetty-client (doto (HttpClient. (ssl-context-factory config))
(.setAddressResolutionTimeout resolve-timeout)
(.setConnectTimeout connect-timeout)
(.setMaxConnectionsPerDestination max-connections-per-destination)
;; these are Jetty defaults, except for the queue size
(.setExecutor (doto (org.eclipse.jetty.util.thread.QueuedThreadPool.
200 8 60000
(java.util.concurrent.LinkedBlockingQueue. ^int (* 2 pending-ops-limit)))
(.setDaemon true))))]
;; This is jetty defaults, except daemonizing scheduler
(.setScheduler jetty-client (org.eclipse.jetty.util.thread.ScheduledExecutorScheduler.
(str (-> jetty-client class .getSimpleName) "@" (.hashCode jetty-client) "-scheduler") true))
(when proxy-port
(-> jetty-client
.getProxyConfiguration
.getProxies
(.add (Socks4Proxy.
(or ^String proxy-host
"localhost")
^int proxy-port))))
(.start jetty-client)
(Client.
jetty-client
(atom 0)
pending-ops-limit))) Note that it takes (-> jetty-client
.getProxyConfiguration
.getProxies
(.add (HttpProxy. ^String proxy-host ^int proxy-port))) Then make a copy of (defn custom-http-client
[]
(let [c (-> (if (and proxy-host proxy-port)
{:proxy-host proxy-host
:proxy-port proxy-port}
{})
create)]
(reify aws/HttpClient
(-submit [_ request channel]
(impl/submit c request channel))
(-stop [_]
(impl/stop c))))) And set the (def ssm-client
(aws/client {:api :ssm
:credentials-provider (cognitect-process-creds-provider)
:http-client (custom-http-client)
:region "us-west-2"})) |
Thank you so much for posting this! |
Oh, and you'll want to create one instance of the HTTP client for multiple AWS clients to share, like the library does. E.g. (def http-client (custom-http-client))
(def ssm-client
(aws/client {:api :ssm
:credentials-provider (cognitect-process-creds-provider)
:http-client http-client
:region "us-west-2"}))
(defn ec2-client
[account-id region]
(aws/client {:api :ec2
:credentials-provider (cognitect-ssm-creds-provider account-id)
:http-client http-client
:region region})) Creating a unique HTTP client per AWS client is a quick way to create lots more threads than you intended and give yourself an OOM error 🙃. |
I recently updated to Jetty 11 using the guides @xiongtx posted above, with the following modifications:
{:deps
{com.cognitect.aws/api {:mvn/version "0.8.686"}
com.cognitect/http-client {:mvn/version "1.0.111"}
org.eclipse.jetty/jetty-client {:mvn/version "11.0.15"}
org.eclipse.jetty/jetty-http {:mvn/version "11.0.15"}
org.eclipse.jetty/jetty-util {:mvn/version "11.0.15"}}}
41 (doto (Log/getProperties) I guess The other issue is And then it seems to work. |
@dchelimsky has there been any progress on this since your last update? |
Hey @gpind. I'm no longer involved with this project. I think @scottbale might be able to answer your question. |
Hi @gpind (and @dchelimsky). Removing the Jetty 9.x dependency is our most immediate goal. Nothing has been released yet. But we have been working on a JDK 11-based http client which has no dependency on Jetty. Potentially it will entirely replace the cognitect http client, thus removing the Jetty 9.x transitive dependency. We are a small team and things have slowed down in December, but I expect them to pick up again starting this month. |
@scottbale got it, and thank you for the update! |
@gpind FYI, we vendored cognitect.http-client to workaround this issue as commented above. main...green-labs:aws-api:http-client in order to update ring-1.11.0, I couldn't put this off any longer. So far so good without any issues with this fix. |
I have a fork of |
Thanks, @tobias! I have a fork of |
We have just released a beta release which introduces a Java-native http client. With this release, Jetty is no longer by default a transitive dependency. @nniv-r7 if time permits, could you please verify whether this release solves your issue? Thanks. (I will leave this issue open for the time being until we verify.) |
Problem/Feature Description
There is no obvious way (at least to me) to use aws-api library with Jetty client that is later than version 9.x.
When instantiating an HTTP client, function cognitect.http-client/ssl-context-factory calls the Jetty constructor of SslContextFactory.
While this works with Jetty 9.x, in Jetty 10.x this class became abstract, so (depending on the Jetty version) the call should be to the constructor of the subclass, i.e.: (SslContextFactory$Client. false). Otherwise the client creation fails with:
Execution error (InstantiationError) at cognitect.http-client/ssl-context-factory (http_client.clj:253).
org.eclipse.jetty.util.ssl.SslContextFactory
Suggestions
One possible way to solve this in a generic manner is to extract the call to the SslContextFactory (or SslContextFactory$Client) constructor from the ssl-context-factory function, either to some optional function parameter or to a var that can be rebound by library users, as needed.
If there is a way to use the library as-is with Jetty client that is later than version 9.x., perhaps it should be documented.
Thanks.
The text was updated successfully, but these errors were encountered: