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

Add options for ReferenceStore #155

Merged
merged 4 commits into from
Feb 25, 2024
Merged

Conversation

keller-mark
Copy link
Collaborator

@keller-mark keller-mark commented Feb 24, 2024

This pull request adds two options for the ReferenceStore:

  • overrides to specify fetch options like auth headers
    • Should these same overrides also be used in the fetch call within ReferenceStore.fromUrl?
  • preferTarget to denote that the opts.target URL should be used regardless of whether a URL is present in the reference JSON
    • I would also be open to removing this change if it is unfavorable as it can alternatively be achieved in user-land by the user manually null-ing the URLs within the contents of the ReferenceEntrys before passing into fromSpec

@manzt
Copy link
Owner

manzt commented Feb 25, 2024

I would also be open to removing this change if it is unfavorable as it can alternatively be achieved in user-land by the user manually null-ing the URLs within the contents of the ReferenceEntrys before passing into fromSpec

Handling a special null to fallback to a target is already a bit of a deviation from the reference specification. I'm open to adding something like preferTarget, but worry dynamically trying to handle entries is challenging and likely hard to handle edge cases with a consistent API. I think I'd rather encourage "pre-processing" an existing spec if this behavior is desired (but open to being convinced otherwise).

One thing that comes to mind is that file references can have multiple URLs (e.g., a Zarr could virtually combine several TIFFs). Right now target only overrides if an entry is null, but in a reference where there is null and other urls it could get complicated.

Maybe a solution would be to make refs public, and let someone modify the refs:

let store = await ReferenceStore.fromUrl("https://example.com/spec.json", {
  target: "http://example.com/some-other-target.tiff",
}) 
for (let entry of refs.values()) {
  // replace all URL refs with 'null', allowing fallback to a new target
  if (Array.isArray(entry)) entry[0] = null; 
}

or have a method to let modifying the refs when the store it first created:

let store = await ReferenceStore.fromUrl("https://example.com/spec.json", {
  target: "http://example.com/some-other-target.tiff",
  preprocess(refs) {
    for (let entry of store.refs.values()) {
      // replace all URL refs with 'null', allowing fallback to a new target
      if (Array.isArray(entry)) entry[0] = null; 
    }
  }
});

@keller-mark
Copy link
Collaborator Author

I see, I hadn't considered those edge cases, I think then best to leave up to the user prior to fromSpec. I have removed that change here

@manzt manzt merged commit 511dbdc into manzt:main Feb 25, 2024
2 checks passed
@github-actions github-actions bot mentioned this pull request Feb 25, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants