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

Feature ask client side -- Surface PackageType query parameter in the NuGet.org search API #8915

Open
wli3 opened this issue Dec 10, 2019 · 10 comments · May be fixed by NuGet/NuGet.Client#5991
Labels
Area:Protocol Client/Server protocol /code around it Category:Quality Week Issues that should be considered for quality week Functionality:SDK The NuGet client packages published to nuget.org Functionality:Search help wanted Considered good issues for community contributions. Partner:CLI-SDK Priority:2 Issues for the current backlog. Type:Feature

Comments

@wli3
Copy link

wli3 commented Dec 10, 2019

NuGet/NuGetGallery#7594

SDK team has the nuget gallery ask. And Peter Yu and @joelverhagen suggested it is better have a client side counterpart instead of directly query the web API. So all client side .NET code is in nuget client.

However, in the meanwhile SDK will continue directly using the web REST API since it is relatively simple, and we will migrate to use the client once it is available.

@nkolev92 nkolev92 added Area:Protocol Client/Server protocol /code around it Functionality:Search Type:Feature labels Dec 11, 2019
@nkolev92 nkolev92 added this to the Backlog milestone Dec 11, 2019
@nkolev92
Copy link
Member

Related to #5725. :)

@wli3
Copy link
Author

wli3 commented Jan 10, 2020

Do we have a decision from the triage?

@nkolev92
Copy link
Member

@wli3 We haven't had a discussion (holidays and all).
fyi @aortiz-msft

@zivkan
Copy link
Member

zivkan commented Jul 20, 2020

This came up in team chat today. We'll need to add an extra property in our search options. However, the question of running search on an old server that does not implement this new search option came up.

From an API design point of view, how should NuGet.Protocol behave when the server implements an old version of the NuGet HTTP protocol and doesn't support packageType filtering?

  • Throw an exception?
    • Also add a TrySearch that returns false when the server doesn't support a filter option (need to also figure out how to communicate which filter option wasn't supported, so this is extensible in the future)?
    • Expect the caller to know how to get the service index resource and check the search service version themselves, plus the version they care about?
    • Add methods or properties to check for capabilies? (SearchResource.SupportsPackageType)
  • Silently ignore the issue, make the request anyway, and return results that may not be the package type requested?

API design is hard :(

@joelverhagen
Copy link
Member

You could consider leveraging the resource provider pattern and make a dedicated resource (which could inherit an existing resource, if you want) and resource provider. If the repository returns null that means the feature is not supported. Just another option to toss into the hat.

@zivkan zivkan added Category:Customer Sprint Priority:1 High priority issues that must be resolved in the current sprint. and removed Priority:2 Issues for the current backlog. labels Nov 17, 2020
@nkolev92 nkolev92 added Category:Quality Week Issues that should be considered for quality week and removed Category:Customer Sprint labels Aug 29, 2022
@nkolev92 nkolev92 added Priority:2 Issues for the current backlog. help wanted Considered good issues for community contributions. and removed Priority:1 High priority issues that must be resolved in the current sprint. labels Nov 17, 2022
@nkolev92 nkolev92 removed this from the Backlog milestone Nov 17, 2022
@glopesdev
Copy link

I am confused about the current API surface. There is a class SearchFilter which includes a PackageTypes property. This is used directly by PackageSearchResourceV3 to build the query string with packageTypeFilter as below:

https://github.com/NuGet/NuGet.Client/blob/0539e8e2516f708c834cb606340c7c9a110f96b3/src/NuGet.Core/NuGet.Protocol/Resources/PackageSearchResourceV3.cs#L145

Strangely, packageTypeFilter doesn't seem to be used anywhere on the server side, where packageType is the default instead. Is there an intended and accepted use for the PackageTypes property in SearchFilter, or is it a faulty implementation of this issue?

@nkolev92
Copy link
Member

packageTypeFilter predates search by package type.
It's probably just dead code client side.

@glopesdev
Copy link

glopesdev commented Aug 19, 2024

@nkolev92 that makes sense. In that case am I understanding correctly that all the API design for this is already made, and the only thing needed would be replacing packageTypeFilter with packageType in the client code?

For consistency I guess it would be nice if package type filtering also applied automatically to local package sources.

@nkolev92
Copy link
Member

The protocol is decided and documented: https://learn.microsoft.com/en-us/nuget/api/search-query-service-resource#search-for-packages.

We're just not using it client side yet, so it's not part of our libraries.

@fforjan
Copy link

fforjan commented Nov 24, 2024

@glopesdev or @nkolev92 any update on this ?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Area:Protocol Client/Server protocol /code around it Category:Quality Week Issues that should be considered for quality week Functionality:SDK The NuGet client packages published to nuget.org Functionality:Search help wanted Considered good issues for community contributions. Partner:CLI-SDK Priority:2 Issues for the current backlog. Type:Feature
Projects
None yet
Development

Successfully merging a pull request may close this issue.

7 participants