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

Check storage constraints on package version for semVerLevel=2 #3561

Closed
xavierdecoster opened this issue Feb 13, 2017 · 7 comments
Closed

Check storage constraints on package version for semVerLevel=2 #3561

xavierdecoster opened this issue Feb 13, 2017 · 7 comments

Comments

@xavierdecoster
Copy link
Member

xavierdecoster commented Feb 13, 2017

Should we define a new constraint? or consolidate on string-length for package version?

Gallery has a StringLength(64) constraint on Package Version: https://github.com/NuGet/NuGetGallery/blob/master/src/NuGetGallery.Core/Entities/Package.cs#L137

CDN stats database supports NVARCHAR (128) https://github.com/NuGet/NuGet.Jobs/blob/master/src/Stats.Warehouse/Tables/dbo.Dimension_Package.sql#L4

@xavierdecoster
Copy link
Member Author

Additional relevant notes:

  • The release label limit of 20 characters was removed in NuGet 4.0.0.
  • The Version field will also start to contain metadata parts (NormalizedVersion field doesn't/not affected) when we enable SemVer v2.0.0 support.

@xavierdecoster
Copy link
Member Author

@emgarten @joelverhagen it looks like the current 64 characters limit for the original version string may need to be increased to adopt semver2 versioning support. We also may need to increase the NormalizedVersion field to support the same max-length. What would be a good new max-length limit for the original version string?

The release label limit of 20 chars has already been lifted client-side, and we'll also need to support the semver2 metadata part in the original version string. The current maximum original package version string length in the gallery database is already 58 characters, without the above.

@xavierdecoster
Copy link
Member Author

xavierdecoster commented Mar 20, 2017

On the other hand, we may also just decide that 64 characters is still a sane default, and decide to increase later if this becomes too much of a problem. The package versions that currently are nearing the 64 characters limit are those that append the full commit SHA to their version string as part of the release label. This typically should end up in the metadata part, so adding the + character in that version string is only an increment of 1 in original Version string length (if it not already replaces a -), and most likely a reduction in length for NormalizedVersion.

@emgarten
Copy link
Member

64 sounds like a sane default for the normalized version string to me. For the full string with metadata I think it makes sense for this to be higher, but still limited.

I agree with starting small and increasing it as we get feedback and see how people are using semver 2.0.0 on nuget.org.

@xavierdecoster
Copy link
Member Author

Thanks @emgarten! I'm fine with keeping it as-is for now, and re-evaluate as needed. @joelverhagen would you agree?

@joelverhagen
Copy link
Member

The database will need to contain the full version string right? That is, normalized version plus any build metadata so that it can be displayed in the web UI. Additionally, the <d:Version> property needs to have the full version string too (original string maybe)?

Given that the example for build metadata is a Git hash, this means the non-normalized version string has the potential to get a lot longer.

That being said, keeping 64 as the max today seems just fine. We can totally react to user feedback on this in the future.

@joelverhagen would you agree?

So, yeah!

@xavierdecoster
Copy link
Member Author

Closing for now, will reopen when needed.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

3 participants