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

Candidate uploads allows version overwrites #69

Open
edsko opened this Issue Sep 24, 2013 · 5 comments

Comments

4 participants
@edsko
Contributor

edsko commented Sep 24, 2013

The official documentation states that if you want to upload another candidate, it must have a different version number. This assumption might be in various places of the code (for instance, it is assumed in the build client that if the docs for a package version have failed then they will forever fail). However, the site currently allows to upload another candidate with the same version number.

@gracenotes

This comment has been minimized.

Show comment
Hide comment
@gracenotes

gracenotes Sep 24, 2013

Contributor

Which documentation? It might be in the wrong here. Candidates exist in different and slightly more volatile namespace. However, it doesn't seem like hooks exist to wipe the state you're talking about.

Contributor

gracenotes commented Sep 24, 2013

Which documentation? It might be in the wrong here. Candidates exist in different and slightly more volatile namespace. However, it doesn't seem like hooks exist to wipe the state you're talking about.

@edsko

This comment has been minimized.

Show comment
Hide comment
@edsko

edsko Sep 24, 2013

Contributor

Well, the "Upload" page says "You can have multiple candidates for each package at the same time so long as they each have different versions". That may or may not imply what I said above.

Regarding hooks for wiping the state -- those hooks cannot exist, because that state (which packages did and did not fail to build) does not exist on the server, but is local state to the doc builder client.

Contributor

edsko commented Sep 24, 2013

Well, the "Upload" page says "You can have multiple candidates for each package at the same time so long as they each have different versions". That may or may not imply what I said above.

Regarding hooks for wiping the state -- those hooks cannot exist, because that state (which packages did and did not fail to build) does not exist on the server, but is local state to the doc builder client.

@gracenotes

This comment has been minimized.

Show comment
Hide comment
@gracenotes

gracenotes Sep 24, 2013

Contributor

Okay, I see. You can have multiple candidates for a particular version, but only serially, and there is only one available "latest" such candidate. The main difference between candidates and the main index, from the perspective of clients, is that immutability is just not a safe thing to assume for the former.

Contributor

gracenotes commented Sep 24, 2013

Okay, I see. You can have multiple candidates for a particular version, but only serially, and there is only one available "latest" such candidate. The main difference between candidates and the main index, from the perspective of clients, is that immutability is just not a safe thing to assume for the former.

@edsko

This comment has been minimized.

Show comment
Hide comment
@edsko

edsko Sep 24, 2013

Contributor

If that is in fact the intended behaviour then the build client must be modified to not store "failed" for package candidates (but that would mean they will be built over and over again) -- perhaps we should store a latest upload date or something.

Contributor

edsko commented Sep 24, 2013

If that is in fact the intended behaviour then the build client must be modified to not store "failed" for package candidates (but that would mean they will be built over and over again) -- perhaps we should store a latest upload date or something.

@lfairy

This comment has been minimized.

Show comment
Hide comment
@lfairy

lfairy Sep 8, 2014

Contributor

It's probably wrong for the build client to assume a failing package will forever fail, even without version overwrites.

For example: consider package A depends on package B. Unfortunately, when the build client attempts package A, package B hasn't been uploaded yet (maybe the maintainer had forgotten). So package A is blacklisted, even though it should build successfully once B is released.

This kind of mistake is easy to avoid (upload dependencies first) but is currently near-impossible to fix when it does happen.

Another case to consider: we currently allow users to edit the Cabal metadata of existing packages. That could turn a failing version into a passing one as well.

Anyway, I second @edsko's solution: store the upload time along with the package ID. The server already returns an accurate Last-modified header so it's definitely viable.

This also gives us an easy way to force a rebuild. If we reset a package's timestamp, the build client will treat it as "modified" and attempt it again.

Contributor

lfairy commented Sep 8, 2014

It's probably wrong for the build client to assume a failing package will forever fail, even without version overwrites.

For example: consider package A depends on package B. Unfortunately, when the build client attempts package A, package B hasn't been uploaded yet (maybe the maintainer had forgotten). So package A is blacklisted, even though it should build successfully once B is released.

This kind of mistake is easy to avoid (upload dependencies first) but is currently near-impossible to fix when it does happen.

Another case to consider: we currently allow users to edit the Cabal metadata of existing packages. That could turn a failing version into a passing one as well.

Anyway, I second @edsko's solution: store the upload time along with the package ID. The server already returns an accurate Last-modified header so it's definitely viable.

This also gives us an easy way to force a rebuild. If we reset a package's timestamp, the build client will treat it as "modified" and attempt it again.

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