[PR #646] [CLOSED] Refactor ResourceReader to support multiple schemes #656

New Issue

2025-12-30T01:25:58+01:00

adam commented

2025-12-30 01:25:58 +01:00

📋 Pull Request Information

Original PR: https://github.com/apple/pkl/pull/646
Author: @HT154
Created: 9/15/2024
Status: ❌ Closed

Base: main ← Head: resource-reader-multiple-schemes

📝 Commits (1)

7e9c98e Refactor ResourceReader to support multiple schemes

📊 Changes

5 files changed (+39 additions, -27 deletions)

View changed files

📝 pkl-core/src/main/java/org/pkl/core/resource/ResourceReader.java (+2 -1)
📝 pkl-core/src/main/java/org/pkl/core/resource/ResourceReaders.java (+19 -18)
📝 pkl-core/src/main/java/org/pkl/core/runtime/ResourceManager.java (+16 -6)
📝 pkl-core/src/test/kotlin/org/pkl/core/resource/TestResourceReader.kt (+1 -1)
📝 pkl-server/src/main/kotlin/org/pkl/server/ClientResourceReader.kt (+1 -1)

📄 Description

This is preparatory work for SPICE-0009. It is being contributed in a separate pull request to ease review.

This is required to allow deferred launch of external reader processes without requiring up-front configuration of scheme->process mappings. More details on why this is required here: https://github.com/apple/pkl-evolution/pull/10#discussion_r1724145677

One concern here (which applies before this change as well) is that the semantics of module key factories and resource readers is reversed. For module key factories, the first factory that answers for a URI scheme "wins", while for resource readers, the last reader that answers for a URI scheme "wins". This PR preserves that behavior, but it may be worth reconsidering this design at this time.

This behavior can be observed in practice in ResourceReadersEvaluatorTest.`module path` where ResourceReaders.modulePath is added after the pre-configured ResourceReaders.classPath and takes precedence.

_{🔄 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/646 **Author:** [@HT154](https://github.com/HT154) **Created:** 9/15/2024 **Status:** ❌ Closed **Base:** `main` ← **Head:** `resource-reader-multiple-schemes` --- ### 📝 Commits (1) - [`7e9c98e`](https://github.com/apple/pkl/commit/7e9c98e0db95fae13f4359f697712d40d4277b0f) Refactor ResourceReader to support multiple schemes ### 📊 Changes **5 files changed** (+39 additions, -27 deletions) <details> <summary>View changed files</summary> 📝 `pkl-core/src/main/java/org/pkl/core/resource/ResourceReader.java` (+2 -1) 📝 `pkl-core/src/main/java/org/pkl/core/resource/ResourceReaders.java` (+19 -18) 📝 `pkl-core/src/main/java/org/pkl/core/runtime/ResourceManager.java` (+16 -6) 📝 `pkl-core/src/test/kotlin/org/pkl/core/resource/TestResourceReader.kt` (+1 -1) 📝 `pkl-server/src/main/kotlin/org/pkl/server/ClientResourceReader.kt` (+1 -1) </details> ### 📄 Description This is preparatory work for [SPICE-0009](https://github.com/apple/pkl-evolution/pull/10). It is being contributed in a separate pull request to ease review. This is required to allow deferred launch of external reader processes without requiring up-front configuration of scheme->process mappings. More details on why this is required here: https://github.com/apple/pkl-evolution/pull/10#discussion_r1724145677 One concern here (which applies before this change as well) is that the semantics of module key factories and resource readers is reversed. For module key factories, the first factory that answers for a URI scheme "wins", while for resource readers, the _last_ reader that answers for a URI scheme "wins". This PR preserves that behavior, but it may be worth reconsidering this design at this time. This behavior can be observed in practice in <code>ResourceReadersEvaluatorTest.\`module path\`</code> where `ResourceReaders.modulePath` is added after the pre-configured `ResourceReaders.classPath` and takes precedence. --- <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:25:58 +01:00

adam closed this issue

2025-12-30 01:25:59 +01:00

adam referenced this issue

2025-12-30 01:25:59 +01:00

[PR #657] [MERGED] Handle `null` contents gracefully in message passing API #658

Sign in to join this conversation.

1 Participants

Notifications

Due Date

No due date set.

Dependencies

No dependencies set.

Reference: starred/pkl#656