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

yanks might get ignored when the build is still queued #1934

Open
syphar opened this issue Nov 27, 2022 · 3 comments
Open

yanks might get ignored when the build is still queued #1934

syphar opened this issue Nov 27, 2022 · 3 comments
Labels
A-builds Area: Building the documentation for a crate C-bug Category: This is a bug E-easy Effort: Should be easy to implement and would make a good first PR

Comments

@syphar
Copy link
Member

syphar commented Nov 27, 2022

So it's not forgotten I'll write down my findings here.

When we get a Yank change from crates-index-diff, we are running an UPDATE statement on the releases table.

We only add the release to that table after the build is finished, so when the release is still in our build-queue, or in progress, the yank wouldn't do anything.

Since we don't check the number of updated rows for that UPDATE statement, this also never lead to an error.

While we could solve this issue by storing the yank-state in the build-queue too, this could also be solved via solving #1011.

@syphar syphar added C-bug Category: This is a bug E-easy Effort: Should be easy to implement and would make a good first PR A-builds Area: Building the documentation for a crate labels Dec 1, 2022
@sentry-io
Copy link

sentry-io bot commented Dec 1, 2022

Sentry issue: DOCS-RS-BACKEND-35

@syphar
Copy link
Member Author

syphar commented Dec 5, 2022

This definitely happens regularly, probably more often when our queue is longer:

grafik

@syphar
Copy link
Member Author

syphar commented Jan 7, 2023

small update here:
While digging into this issue I discovered one thing:

after a build, we're already fetching the release-data from crates.io, and updating our release-record with the yank-status from the response.

let release_data = match self.index.api().get_release_data(name, version) {
Ok(data) => data,
Err(err) => {
warn!("{:#?}", err);
ReleaseData::default()
}
};

so I created a PR to

  • change a warn to error when the crates.io data can't be fetched.
  • only log an error for the yank-state-update if we don't have a build queued, assuming the build will fix the yank-state

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A-builds Area: Building the documentation for a crate C-bug Category: This is a bug E-easy Effort: Should be easy to implement and would make a good first PR
Projects
None yet
Development

No branches or pull requests

1 participant