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

internal/searcher,lock: use /v2/resolve endpoint #1790

Merged
merged 1 commit into from
Feb 7, 2024
Merged

Conversation

gcurtis
Copy link
Collaborator

@gcurtis gcurtis commented Feb 5, 2024

Add a DEVBOX_FEATURE_RESOLVE_V2 feature flag that enables the new /v2/resolve endpoint when resolving packages. The new endpoint unlocks a couple of improvements:

  • We no longer need to call nix store path-from-hash-part (which incurs a network request) for every fetchClosure package. The full store path is returned directly by the API.
  • The resolved package response includes store paths for all known outputs, allowing us to support fetchClosure for non-default outputs as well.
  • The new endpoint returns a flake installable rather than a nixpkgs commit hash and attribute path. This gives more flexibility, allowing the API to potentially return packages from sources other than nixpkgs.

Add a `DEVBOX_FEATURE_RESOLVE_V2` feature flag that enables the new
`/v2/resolve` endpoint when resolving packages. The new endpoint
unlocks a couple of improvements:

- We no longer need to call `nix store path-from-hash-part` (which
  incurs a network request) for every `fetchClosure` package. The full
  store path is returned directly by the API.
- The resolved package response includes store paths for all known
  outputs, allowing us to support `fetchClosure` for non-default outputs
  as well.
- The new endpoint returns a flake installable rather than a nixpkgs
  commit hash and attribute path. This gives more flexibility, allowing
  the API to potentially return packages from sources other than
  nixpkgs.
Copy link
Contributor

@mikeland73 mikeland73 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Would be nice to add an additional test run to matrix with this flag enabled that way we can start seeing tests (and perf implication if there is any) right away.

Comment on lines +44 to +46

// ResolveResponse is a response from the /v2/resolve endpoint.
type ResolveResponse struct {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Would be nice to share these with search service if possible.

@gcurtis
Copy link
Collaborator Author

gcurtis commented Feb 7, 2024

Would be nice to add an additional test run to matrix with this flag enabled that way we can start seeing tests (and perf implication if there is any) right away.

Agreed. I can do that once the backend change is deployed.

@gcurtis gcurtis merged commit 81dfaac into main Feb 7, 2024
24 checks passed
@gcurtis gcurtis deleted the gcurtis/resolve-v2 branch February 7, 2024 20:09
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Development

Successfully merging this pull request may close these issues.

2 participants