starred/pkl
1
0
Fork 0
mirror of https://github.com/apple/pkl.git synced 2026-01-11 22:30:54 +01:00
Code Issues 198 Packages Projects Releases 10 Wiki Activity

[PR #217] [MERGED] Use java.net.http.HttpClient instead of java.net.Http(s)URLConnection #458

New Issue
Closed
opened 2025-12-30 01:24:36 +01:00 by adam · 0 comments
adam commented 2025-12-30 01:24:36 +01:00
Owner
Copy Link

📋 Pull Request Information

Original PR: https://github.com/apple/pkl/pull/217
Author: @odenix
Created: 2/20/2024
Status: ✅ Merged
Merged: 3/6/2024
Merged by: @bioball

Base: main ← Head: http-client

📝 Commits (10+)

  • 9ac214a Use java.net.http.HttpClient instead of java.net.Http(s)URLConnection
  • dc78962 Support CA certificates in Executor API
  • 903f141 Build CLI HTTP client lazily
  • 1ca9814 Improve handling of HTTP client initialization errors
  • 765b439 Improve SSL related error messages
  • 988481f Rename class
  • 0022ea3 Rename some internals
  • 0b3734d Remove assert that is specific to JDK implementation
  • 8f8ac86 Fix compile error on JDK 11
  • b12db60 Incorporate review feedback

📊 Changes

79 files changed (+2376 additions, -395 deletions)

View changed files

📝 bench/src/jmh/java/org/pkl/core/ListSort.java (+2 -0)
📝 docs/src/test/kotlin/DocSnippetTests.kt (+2 -0)
📝 gradle/libs.versions.toml (+1 -0)
➕ pkl-certs/pkl-certs.gradle.kts (+19 -0)
📝 pkl-certs/src/main/resources/org/pkl/certs/PklCARoots.pem (+0 -0)
📝 pkl-cli/pkl-cli.gradle.kts (+1 -1)
📝 pkl-cli/src/main/kotlin/org/pkl/cli/CliPackageDownloader.kt (+1 -1)
📝 pkl-cli/src/main/kotlin/org/pkl/cli/CliProjectPackager.kt (+1 -0)
📝 pkl-cli/src/main/kotlin/org/pkl/cli/CliProjectResolver.kt (+1 -0)
📝 pkl-cli/src/main/kotlin/org/pkl/cli/CliRepl.kt (+1 -0)
📝 pkl-cli/src/main/kotlin/org/pkl/cli/CliServer.kt (+1 -1)
📝 pkl-cli/src/test/kotlin/org/pkl/cli/CliEvaluatorTest.kt (+45 -13)
📝 pkl-cli/src/test/kotlin/org/pkl/cli/CliPackageDownloaderTest.kt (+1 -1)
📝 pkl-cli/src/test/kotlin/org/pkl/cli/CliProjectPackagerTest.kt (+8 -5)
📝 pkl-cli/src/test/kotlin/org/pkl/cli/repl/ReplMessagesTest.kt (+2 -0)
📝 pkl-commons-cli/pkl-commons-cli.gradle.kts (+1 -0)
📝 pkl-commons-cli/src/main/kotlin/org/pkl/commons/cli/CliBaseOptions.kt (+29 -6)
📝 pkl-commons-cli/src/main/kotlin/org/pkl/commons/cli/CliCommand.kt (+6 -7)
📝 pkl-commons-cli/src/main/kotlin/org/pkl/commons/cli/commands/BaseOptions.kt (+2 -31)
📝 pkl-commons-test/pkl-commons-test.gradle.kts (+1 -0)

...and 59 more files

📄 Description

Moving to java.net.http.HttpClient brings many benefits, including HTTP/2 support and the ability to make asynchronous requests.

Major additions and changes:

  • Introduce a lightweight org.pkl.core.http.HttpClient API. This keeps some flexibility and allows to enforce behavior such as setting the User-Agent header.
  • Provide an implementation that delegates to java.net.http.HttpClient.
  • Use HttpClient for all HTTP(s) requests across the codebase. This required adding an HttpClient parameter to constructors and factory methods of multiple classes, some of which are public APIs.
  • Manage CA certificates per HTTP client instead of per JVM. This makes it unnecessary to set JVM-wide system/security properties and default SSLSocketFactory's.

Each HTTP client maintains its own connection pool and SSLContext. For efficiency reasons, I've tried to reuse clients whenever feasible. To avoid memory leaks, clients are not stored in static fields.

HTTP clients are expensive to create. For this reason, EvaluatorBuilder defaults to a "lazy" client that creates the underlying java.net.http.HttpClient on the first send (which may never happen).

Fixes #157.

Note: This PR allows to fix the "flaky PackageServer tests" problem in a principled way. Because all HTTP requests are now routed through HttpClient, PackageServer can bind to a dynamic port, and HttpClient can rewrite requests to use that port in test mode. I've tested this approach on a local branch, and it's the first time I can get "gw clean build" to pass without "connection refused" errors.

🔄 This issue represents a GitHub Pull Request. It cannot be merged through Gitea due to API limitations.

## 📋 Pull Request Information **Original PR:** https://github.com/apple/pkl/pull/217 **Author:** [@odenix](https://github.com/odenix) **Created:** 2/20/2024 **Status:** ✅ Merged **Merged:** 3/6/2024 **Merged by:** [@bioball](https://github.com/bioball) **Base:** `main` ← **Head:** `http-client` --- ### 📝 Commits (10+) - [`9ac214a`](https://github.com/apple/pkl/commit/9ac214ad0ef9f767787766dbc9eb4621a4ad1f3f) Use java.net.http.HttpClient instead of java.net.Http(s)URLConnection - [`dc78962`](https://github.com/apple/pkl/commit/dc789627e278701b0e50fef9528a2bc6568d331a) Support CA certificates in Executor API - [`903f141`](https://github.com/apple/pkl/commit/903f1415a70ee5553717e8c1ab58f2879c9e8b45) Build CLI HTTP client lazily - [`1ca9814`](https://github.com/apple/pkl/commit/1ca9814b601f95f74c53a19f93024ad355129b8c) Improve handling of HTTP client initialization errors - [`765b439`](https://github.com/apple/pkl/commit/765b439d26f286f006c27a8a35f3b14c206ac72f) Improve SSL related error messages - [`988481f`](https://github.com/apple/pkl/commit/988481f4674c239eaa0a166d1047870f9991ea6d) Rename class - [`0022ea3`](https://github.com/apple/pkl/commit/0022ea38368275d18bb2df6d13d5017525b09232) Rename some internals - [`0b3734d`](https://github.com/apple/pkl/commit/0b3734d9a1c387be78ce6896ed1c881bc6d2d482) Remove assert that is specific to JDK implementation - [`8f8ac86`](https://github.com/apple/pkl/commit/8f8ac8660bf02448778affae6aacea33765ff0ac) Fix compile error on JDK 11 - [`b12db60`](https://github.com/apple/pkl/commit/b12db6073982d45003a6c74b1f4fd9f50c917b36) Incorporate review feedback ### 📊 Changes **79 files changed** (+2376 additions, -395 deletions) <details> <summary>View changed files</summary> 📝 `bench/src/jmh/java/org/pkl/core/ListSort.java` (+2 -0) 📝 `docs/src/test/kotlin/DocSnippetTests.kt` (+2 -0) 📝 `gradle/libs.versions.toml` (+1 -0) ➕ `pkl-certs/pkl-certs.gradle.kts` (+19 -0) 📝 `pkl-certs/src/main/resources/org/pkl/certs/PklCARoots.pem` (+0 -0) 📝 `pkl-cli/pkl-cli.gradle.kts` (+1 -1) 📝 `pkl-cli/src/main/kotlin/org/pkl/cli/CliPackageDownloader.kt` (+1 -1) 📝 `pkl-cli/src/main/kotlin/org/pkl/cli/CliProjectPackager.kt` (+1 -0) 📝 `pkl-cli/src/main/kotlin/org/pkl/cli/CliProjectResolver.kt` (+1 -0) 📝 `pkl-cli/src/main/kotlin/org/pkl/cli/CliRepl.kt` (+1 -0) 📝 `pkl-cli/src/main/kotlin/org/pkl/cli/CliServer.kt` (+1 -1) 📝 `pkl-cli/src/test/kotlin/org/pkl/cli/CliEvaluatorTest.kt` (+45 -13) 📝 `pkl-cli/src/test/kotlin/org/pkl/cli/CliPackageDownloaderTest.kt` (+1 -1) 📝 `pkl-cli/src/test/kotlin/org/pkl/cli/CliProjectPackagerTest.kt` (+8 -5) 📝 `pkl-cli/src/test/kotlin/org/pkl/cli/repl/ReplMessagesTest.kt` (+2 -0) 📝 `pkl-commons-cli/pkl-commons-cli.gradle.kts` (+1 -0) 📝 `pkl-commons-cli/src/main/kotlin/org/pkl/commons/cli/CliBaseOptions.kt` (+29 -6) 📝 `pkl-commons-cli/src/main/kotlin/org/pkl/commons/cli/CliCommand.kt` (+6 -7) 📝 `pkl-commons-cli/src/main/kotlin/org/pkl/commons/cli/commands/BaseOptions.kt` (+2 -31) 📝 `pkl-commons-test/pkl-commons-test.gradle.kts` (+1 -0) _...and 59 more files_ </details> ### 📄 Description Moving to java.net.http.HttpClient brings many benefits, including HTTP/2 support and the ability to make asynchronous requests. Major additions and changes: - Introduce a lightweight org.pkl.core.http.HttpClient API. This keeps some flexibility and allows to enforce behavior such as setting the User-Agent header. - Provide an implementation that delegates to java.net.http.HttpClient. - Use HttpClient for all HTTP(s) requests across the codebase. This required adding an HttpClient parameter to constructors and factory methods of multiple classes, some of which are public APIs. - Manage CA certificates per HTTP client instead of per JVM. This makes it unnecessary to set JVM-wide system/security properties and default SSLSocketFactory's. Each HTTP client maintains its own connection pool and SSLContext. For efficiency reasons, I've tried to reuse clients whenever feasible. To avoid memory leaks, clients are not stored in static fields. HTTP clients are expensive to create. For this reason, EvaluatorBuilder defaults to a "lazy" client that creates the underlying java.net.http.HttpClient on the first send (which may never happen). Fixes #157. Note: This PR allows to fix the "flaky PackageServer tests" problem in a principled way. Because all HTTP requests are now routed through HttpClient, PackageServer can bind to a dynamic port, and HttpClient can rewrite requests to use that port in test mode. I've tested this approach on a local branch, and it's the first time I can get "gw clean build" to pass without "connection refused" errors. --- <sub>🔄 This issue represents a GitHub Pull Request. It cannot be merged through Gitea due to API limitations.</sub>
adam added the pull-request label 2025-12-30 01:24:36 +01:00
adam closed this issue 2025-12-30 01:24:36 +01:00
Sign in to join this conversation.
No Branch/Tag Specified
Branches Tags
Labels
Clear labels
bug
duplicate
enhancement
good first issue
help wanted
pull-request
Mirrored from GitHub Pull Request
No Label pull-request
Milestone
No items
No Milestone
Projects
Clear projects
No project
Assignees
Clear assignees
adam
No Assignees
1 Participants
Notifications
Due Date
No due date set.
Dependencies

No dependencies set.

Reference: starred/pkl#458
Delete Branch "%!s()"

Deleting a branch is permanent. Although the deleted branch may continue to exist for a short time before it actually gets removed, it CANNOT be undone in most cases. Continue?