Technical implementation proposal for Sponsorship in CLI#14997
Technical implementation proposal for Sponsorship in CLI#14997kalebfik wants to merge 3 commits intodevNuGet/Home:devfrom dev-kalebfika-sponsorTechSpecNuGet/Home:dev-kalebfika-sponsorTechSpecCopy head branch name to clipboard
Conversation
| ```mermaid | ||
| flowchart LR | ||
| A["<b>Gallery Database</b><br/>Stores package state, including sponsorship URLs"] | ||
| A --> B["<b>Catalog Writer Job</b><br/>Builds append-only catalog entries"] |
There was a problem hiding this comment.
I believe mutable properties like owners skip the catalog. I think it's read straight from the DB and compared with prior snapshot.
| **Proposed server flow:** | ||
|
|
||
| - Add a **sub-command**, e.g. `UpdateSponsorshipCommand`, to the existing `Auxiliary2AzureSearchCommand` array. | ||
| - Following the `UpdateOwnersCommand` pattern, read `PackageRegistration.SponsorshipUrls` per package ID from the Gallery database, diff against the last-indexed snapshot, and push partial-document updates to Azure Search for changed IDs only, setting a new `SponsorshipUrls` field on `SearchDocument`. |
There was a problem hiding this comment.
I think you should also mention how the first snapshot is created before this job takes over with comparisons with prior snapshots. I believe it's a separate job that does it.
| ``` | ||
|
|
||
| ### Proposed alternative design: | ||
| A dedicated `SponsorshipUriTemplate` V3 resource with per-package-ID fetch, consumed by a new `--sponsor` flag within `dotnet list package` |
There was a problem hiding this comment.
Our aim with alternative approach is still dotnet package sponsor right?
There was a problem hiding this comment.
Yes! After further conversations, dotnet package sponsor is going to be the command used. The updated spec will reflect thsi
| ```mermaid | ||
| flowchart LR | ||
| A["<b>Gallery Database</b><br/>Stores package state, including sponsorship URLs"] | ||
| A --> B["<b>Catalog Writer Job</b><br/>Builds append-only catalog entries"] |
There was a problem hiding this comment.
Can you confirm if catalog writer job will get involved here?
|
|
||
| #### Proposed server flow: DB → API data flow | ||
|
|
||
| - A new, dedicated **Sponsorship Snapshot Job**: |
There was a problem hiding this comment.
It might be good to mentioned that you want to consider reusing the Auxiliary2AzureSearch job here to get opinions on that and see if it's feasible.
| @@ -1,31 +0,0 @@ | ||
| name: IceBox Upvote Watcher |
There was a problem hiding this comment.
assuming these removes are accidental and can be brought back
|
|
||
| #### Data assumptions | ||
|
|
||
| - The client expects optional `sponsorshipUrls` as `IReadOnlyList<string>`. |
There was a problem hiding this comment.
We could be more specific about what the new json in the registration resource would look like after including this new sponsorshipUrls property
There was a problem hiding this comment.
updating spec to reflect this!
| } | ||
| ``` | ||
|
|
||
| **Ten sponsors** (matching NuGet.org's current policy cap) |
There was a problem hiding this comment.
Call out we're not validating anything client side.,
There was a problem hiding this comment.
Yes, I'd rephrase to say Client is showing whatever the server provides. And as of the time of this spec, that's max 10 URLs.
| - Candidate A extends `ListPackageCommand` with a new report flag. | ||
| - Candidate B registers `dotnet package sponsor` but still routes into the same metadata-fetch, model, and renderer internals. | ||
|
|
||
| Planned code updates: |
There was a problem hiding this comment.
nit: This is a very thorough write-up, and I appreciate the level of detail in the planned code updates. For a design spec, this may be more detail than necessary. Since implementation details can change, including too much code-level detail can make the spec stale over time.
|
|
||
| #### Source transport dependency and client abstraction | ||
|
|
||
| This proposal assumes the CLI consumes sponsorship data from the Registration API. |
There was a problem hiding this comment.
How do we detect that registration supports sponsorship?
I'm sure you have a plan, but just calling it out here would nbe good.
|
|
||
| ## Unresolved Questions | ||
|
|
||
| 1. Which command surface ships: `dotnet package list --sponsor` or `dotnet package sponsor`? |
There was a problem hiding this comment.
If we're not making dotnet package sponsor default to nuget.org only, then I don't mind dotnet package list --sponsor. Seems simnpler.
There was a problem hiding this comment.
The issue with defaults is what's the privacy risk of exposing package IDs to nuget.org unintentionally?
Do we simply respect package source mapping, so if it's enabled, we only check sponsorship for packages that were mapped to "https://api.nuget.org/v3/index.json"?
All other packages IDs mapped to other sources, we say "N/A" or something for the sponsorship URL output?
There was a problem hiding this comment.
I think if we add it to dotnet package list --sponsor then we should implement PSM.
Or at least assume we will implement it at some point.
All other packages IDs mapped to other sources, we say "N/A" or something for the sponsorship URL output?
Assuming we can detect the difference between no sponsorship details and no sponsorship period, then we can say N/A or whatever.
We can also just not show it at all. IMO, I'd just keep it simple and not show it at all.
| ## Unresolved Questions | ||
|
|
||
| 1. Which command surface ships: `dotnet package list --sponsor` or `dotnet package sponsor`? | ||
| 2. Should `--include-transitive` be supported? |
There was a problem hiding this comment.
IMO, yes.
Real question si what's the default.
There was a problem hiding this comment.
If we support --include-transitive, that implies the default is top-level only.
There was a problem hiding this comment.
If we support --include-transitive, that implies the default is top-level only.
Correct.
Maybe we keep it simple and we just show them all?
I honestly don't know if people would sponsor transitive packages and how they'd feel about that.
There was a problem hiding this comment.
Your proposal sounds more like, show all by default, then have --exclude-transitive to only output the top-level?
| 1. Which command surface ships: `dotnet package list --sponsor` or `dotnet package sponsor`? | ||
| 2. Should `--include-transitive` be supported? | ||
| For example, if package A depends on package B, should sponsorship data for package B also be shown? | ||
| 3. Should the command special-case NuGet.org by default? |
There was a problem hiding this comment.
If we default to nuget.org, imo we should choose dotnet package sponsor.
If we don't, then I think both are an option.
| ``` | ||
|
|
||
| ```json | ||
| { |
There was a problem hiding this comment.
Should we have a version for this json
| dotnet package list --sponsor | ||
|
|
||
| # Candidate B | ||
| dotnet package sponsor |
There was a problem hiding this comment.
If we go with this one let's make sure to add an allow insecure connections option like dotnet package list.
There was a problem hiding this comment.
I'd rather have someone ask for this.
The expectation is that this would only be used against nuget.org in practice right?
that should always be https only.
No description provided.