Skip to content

Commit

Permalink
Remove dirty status waiting during montoring
Browse files Browse the repository at this point in the history
There isn't a ton of reason to do this here:

- Uploads now already wait for the dirty status to disappear as part of
  filtering out disabled architectures.
- If the dirty status flips during a build, there's no reason to wait on
  it again: the build is already running!
- The presumed reason for the existence of these checks in our original
  ci-package-builder was to help eliminate the race between sources
  being uploaded and new builds appearing, which we already handle in a
  completely different way.

Signed-off-by: Ryan Gonzalez <[email protected]>
  • Loading branch information
refi64 authored and sjoerdsimons committed Jun 5, 2023
1 parent 42ae801 commit 51684d8
Show file tree
Hide file tree
Showing 2 changed files with 2 additions and 23 deletions.
1 change: 0 additions & 1 deletion src/handler.rs
Original file line number Diff line number Diff line change
Expand Up @@ -938,7 +938,6 @@ mod tests {
log_tail: TEST_LOG_TAIL,
monitor: PackageMonitoringOptions {
sleep_on_building: Duration::ZERO,
sleep_on_dirty: Duration::ZERO,
sleep_on_old_status: OLD_STATUS_SLEEP_DURATION,
// High limit, since we don't really test that
// functionality in the handler tests.
Expand Down
24 changes: 2 additions & 22 deletions src/monitor.rs
Original file line number Diff line number Diff line change
Expand Up @@ -24,7 +24,6 @@ pub enum PackageCompletion {
#[derive(Debug)]
enum PackageBuildState {
PendingStatusPosted,
Dirty,
Building(obs::PackageCode),
Completed(PackageCompletion),
}
Expand All @@ -49,7 +48,6 @@ pub struct MonitoredPackage {
#[derive(Clone, Debug)]
pub struct PackageMonitoringOptions {
pub sleep_on_building: Duration,
pub sleep_on_dirty: Duration,
pub sleep_on_old_status: Duration,
pub max_old_status_retries: usize,
}
Expand All @@ -58,7 +56,6 @@ impl Default for PackageMonitoringOptions {
fn default() -> Self {
PackageMonitoringOptions {
sleep_on_building: Duration::from_secs(10),
sleep_on_dirty: Duration::from_secs(30),
sleep_on_old_status: Duration::from_secs(15),
max_old_status_retries: 40, // 15 seconds * 40 tries = 10 minutes
}
Expand Down Expand Up @@ -118,9 +115,6 @@ impl ObsMonitor {
self.package.arch
)
})?;
if result.dirty {
return Ok(PackageBuildState::Dirty);
}

let status = match result.get_status(&self.package.package) {
Some(status) => status,
Expand All @@ -129,9 +123,7 @@ impl ObsMonitor {
None => return Ok(PackageBuildState::PendingStatusPosted),
};

if status.dirty {
Ok(PackageBuildState::Dirty)
} else if status.code.is_final() {
if status.code.is_final() {
// Similarly to above, there is a small gap after a commit where the
// previous build status is still posted. To ensure the build that's
// now final is actually our own, check the build history to make
Expand Down Expand Up @@ -206,7 +198,6 @@ impl ObsMonitor {

let mut previous_code = None;
let mut old_status_retries = 0;
let mut was_dirty = false;

loop {
let state = self.get_latest_state().await?;
Expand All @@ -224,14 +215,6 @@ impl ObsMonitor {

tokio::time::sleep(options.sleep_on_building).await;
}
PackageBuildState::Dirty => {
if !was_dirty {
outputln!("Package is dirty, trying again later...");
}

was_dirty = true;
tokio::time::sleep(options.sleep_on_dirty).await;
}
PackageBuildState::PendingStatusPosted => {
ensure!(
old_status_retries < options.max_old_status_retries,
Expand All @@ -255,9 +238,6 @@ impl ObsMonitor {
if !matches!(state, PackageBuildState::PendingStatusPosted) {
old_status_retries = 0;
}
if !matches!(state, PackageBuildState::Dirty) {
was_dirty = false;
}
}
}

Expand Down Expand Up @@ -545,7 +525,7 @@ mod tests {
);

let state = assert_ok!(monitor.get_latest_state().await);
assert_matches!(state, PackageBuildState::Dirty);
assert_matches!(state, PackageBuildState::Building(PackageCode::Unknown));

mock.set_package_build_status(
TEST_PROJECT,
Expand Down

0 comments on commit 51684d8

Please sign in to comment.