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

nuget CLI should print the URL to the package page #6333

Open
karann-msft opened this issue Dec 18, 2017 · 11 comments
Open

nuget CLI should print the URL to the package page #6333

karann-msft opened this issue Dec 18, 2017 · 11 comments
Assignees
Labels
Functionality:Push Priority:2 Issues for the current backlog. Type:DCR Design Change Request
Milestone

Comments

@karann-msft
Copy link
Contributor

karann-msft commented Dec 18, 2017

After a package is pushed, it undergoes several async validations. The package details page is the only place where the owner can check the latest status of these validations and if package publishing succeeded. This page becomes more important as we add more validations.
Hence, after a package has been pushed using the CLI (nuget.exe or dotnet), it should also print the URL to the package details page.

cc: @anangaur

The CLI output should read:

C:\Users\karann\Downloads>nuget push AppLogger.1.0.2.nupkg -apikey ----redacted---- -source nuget.org
Pushing AppLogger.1.0.2.nupkg to 'https://www.nuget.org/api/v2/package'...
  PUT https://www.nuget.org/api/v2/package/
  Created https://www.nuget.org/api/v2/package/ 11440ms
Your package was pushed.
Note: The package will appear in search results and will be available for install/restore after both validation and indexing are complete. Package validation and indexing may take up to an hour. You can track the progress on the package page - https://www.nuget.org/packages/AppLogger/1.0.2
@anangaur
Copy link
Member

I recommend addition of the following note:

Note: the package will undergo validations before it becomes available on NuGet.org. You can track the progress on the package page - https://nuget.org/packages/<packageID>/<packageVersion>

@emgarten
Copy link
Member

Will the protocol be changed to return this information? Or is it expected that the client will hardcode this for nuget.org?

@anangaur
Copy link
Member

@emgarten not sure. But doesn't client print the messages from the service? Or is it only for errors?

@emgarten
Copy link
Member

I'm not sure. There is a generic way to print warnings, but those are always warnings.

@anangaur
Copy link
Member

anangaur commented Dec 19, 2017

Worth an investigation. I feel the response should be 202 as per the REST response codes guidelines which states the following:

202 Accepted
The request has been accepted for processing, but the processing has not been completed. The request might or might not eventually be acted upon, as it might be disallowed when processing actually takes place. There is no facility for re-sending a status code from an asynchronous operation such as this.

@mishra14
Copy link
Contributor

I think getting that information from the server seems like the right thing to do here.

cc @joelverhagen for server side input.

@emgarten
Copy link
Member

It looks like 202 may work for nuget.exe 2.8.6.
https://github.com/NuGet/NuGet2/blob/c3d1027a51b31fd0c41e9abbe90810cf1c924c9f/src/Core/Server/PackageServer.cs#L243-L248

However this could impact other services that push packages to nuget.org such as myget and appeveyor which we should consider.

The safest fix would be to make a new endpoint, or to pass a parameter when pushing.

//cc @xavierdecoster @maartenba

@anangaur
Copy link
Member

Isn’t 202 also a success and unless the clients are looking for only 200 and no other response codes, this should be okay. How do older nuget.exe fare?

@anangaur
Copy link
Member

My bad - I didn’t see you already mentioned 2.8.6 :)

@mishra14 mishra14 added Priority:2 Issues for the current backlog. and removed Triage:NeedsTriageDiscussion labels Dec 21, 2017
@anangaur
Copy link
Member

anangaur commented Jan 3, 2018

Just had a discussion with @rrelyea where it dawned upon us that we do not allow push for clients older than 4.1

So I guess it's more about third party clients. Also I think MyGet should not have any issues as they can make the change and since they are a service they do not have to deal with old behavior. The same goes with AppVeyor and Octopus Deploy.

Tagging the usual suspects :)

Paket @forki @isaacabraham
Octopus Deploy @PaulStovell
ProGet @apapadimoulis
NuGet Package Explorer @304NotModified
Chocolatey @ferventcoder
JetBrains TeamCity @ekoshkin
AppVeyor @FeodorFitsner

@joelverhagen
Copy link
Member

For the HTTP 202 issue, I have had the following issue tracked for ages: NuGet/NuGetGallery#3603

Agreed that 202 is more appropriate. Easy server-side fix. We just need to be careful with the clients as you guys have been investigating. Given the 4.1 restriction that @anangaur mentioned, it seems like we should go for it as long as the community doesn't raise any red flags.

Concerning the gallery URL, I can think of the following approaches (there may be others).

  1. (Best, I think) Add a new V3 resource for this, e.g. a URL pattern. There is precedent with the report abuse URL resource. I think this is preferable since it won't cause any issues with alternate client implementations. This does require that the gallery URL has a specific pattern. Suppose the server side uses an integer to identify packages in the gallery URLs (which is not the case on NuGet.org)… it would not be possible to construct a URL using client side knowledge. This is probably okay since in the NuGet domain ID+Version is the identity of a package. One drawback is that V2 pushes will not know about this URL. We can also use this V3 resource to put a gallery URL in the VS UI.

  2. Come up with a schema for the push response body, e.g. {"schemaVersion":1,"galleryUrl":"https://www.nuget.org/packages/NuGet.Frameworks/4.5.0"}. This one isn’t great because it only provides the package author with the URL. Option 1 allows anyone to create the URL. This works for both V2 and V3 pushes.

  3. Provide a Location header on the response. There is some precedent for this in other applications. However, I kind of dislike this one since in a lot of people minds I think Location has come to mean JUST redirect and nothing else. Also, the redirect to gallery UI is kind of arbitrary. It should be anywhere (e.g. the package in the .../packages/ CDN container). There is no canonical resource URL on a V3 NuGet source (in the same way that a well-design REST API/ would provide). Package entities on the V2 (OData) API on NuGet.org has a GalleryDetailsUrl property. Theoretically, we could use an [OData URL](https://www.nuget.org/api/v2/Packages(Id='Newtonsoft.Json',Version='3.5.8') as the value for the Location header but this would require a second hop to get the gallery URL and tie us to V2.

One hack for older clients would be to use the warning header that client observes. This would make the text appear in yellow I think which may not be desired.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Functionality:Push Priority:2 Issues for the current backlog. Type:DCR Design Change Request
Projects
None yet
Development

No branches or pull requests

9 participants