grahamc/hydra2 - Change T3OHZDYPABJBSN2G2BMQ47RL5PI5WVMAYQJVPT6TCPEFVGRLPEFQC

hydra-notify: move BuildFinished processing to an Event

Created by Graham Christensen on August 12, 2021

T3OHZDYPABJBSN2G2BMQ47RL5PI5WVMAYQJVPT6TCPEFVGRLPEFQC

Dependencies

In channels

main

Change contents

Replacement in src/lib/Hydra/Event/BuildFinished.pm at line 21 [5.20]

B:BD[5.563] → [5.563:696]

    my ($self, $build_id, $dependencies) = @_;
    return bless { "build_id" => $build_id, "dependencies" => $dependencies }, $self;

[5.563]

[5.696]

    my ($self, $build_id, $dependent_ids) = @_;
    return bless {
        "build_id" => $build_id,
        "dependent_ids" => $dependent_ids,
        "build" => undef,
        "dependents" => [],
    }, $self;
}
sub load {
    my ($self, $db) = @_;
    if (!defined($self->{"build"})) {
        $self->{"build"} = $db->resultset('Builds')->find($self->{"build_id"})
            or die "build $self->{'build_id'} does not exist\n";
        foreach my $id (@{$self->{"dependent_ids"}}) {
            my $dep = $db->resultset('Builds')->find($id)
                or die "dependent build $id does not exist\n";
            push @{$self->{"dependents"}}, $dep;
        }
    }
}
sub execute {
    my ($self, $db, $plugin) = @_;
    $self->load($db);
    $plugin->buildFinished($self->{"build"}, $self->{"dependents"});
    # Mark the build and all dependents as having their notifications "finished".
    #
    # Otherwise, the dependent builds will remain with notificationpendingsince set
    # until hydra-notify is started, as buildFinished is never emitted for them.
    foreach my $b ($self->{"build"}, @{$self->{"dependents"}}) {
        if ($b->finished && defined($b->notificationpendingsince)) {
            $b->update({ notificationpendingsince => undef })
        }
    }
    return 1;

Insertion in src/script/hydra-notify at line 5 [6.2994]

[6.3053]

use Hydra::Event::BuildFinished;
use Hydra::Helper::AddBuilds;
use Hydra::Helper::Nix;

Deletion in src/script/hydra-notify at line 10 [6.2994]
∅:D[7.1752] → [6.3072:3096]
B:BD[6.3072] → [6.3072:3096]
B:BD[6.3128] → [8.5636:5666]
```
use Hydra::Helper::Nix;
use Hydra::Helper::AddBuilds;
```
Deletion in src/script/hydra-notify at line 47 [6.2994]
B:BD[9.1743] → [9.1743:1744]
B:BD[10.3822] → [9.1922:1942]
B:BD[9.1942] → [11.0:31]
```
sub buildFinished {
    my ($buildId, @deps) = @_;
```

Deletion in src/script/hydra-notify at line 48 [6.2994]

B:BD[11.32] → [11.32:135]

B:BD[8.5892] → [9.1973:1974]

∅:D[12.1823] → [6.3581:3601]

∅:D[9.1974] → [6.3581:3601]

∅:D[8.5892] → [6.3581:3601]

B:BD[6.3581] → [6.3581:3601]

B:BD[6.3601] → [9.1975:2004]

∅:D[9.2004] → [6.3630:3771]

B:BD[6.3630] → [6.3630:3771]

B:BD[6.3771] → [10.3823:3860]

B:BD[10.3860] → [13.858:1033]

∅:D[13.1033] → [10.4000:4016]

B:BD[10.4000] → [10.4000:4016]

B:BD[10.4016] → [9.2005:2006]

B:BD[9.2006] → [2.0:416]

∅:D[2.416] → [10.4016:4018]

∅:D[9.2065] → [10.4016:4018]

B:BD[10.4016] → [10.4016:4018]

∅:D[9.2128] → [14.473:474]

B:BD[14.473] → [14.473:474]

    my $build = $db->resultset('Builds')->find($buildId)
    or die "build $buildId does not exist\n";
    my @dependents;
    foreach my $id (@deps) {
        my $dep = $db->resultset('Builds')->find($id)
            or die "build $id does not exist\n";
        push @dependents, $dep;
    }
    foreach my $plugin (@plugins) {
        eval {
            $plugin->buildFinished($build, [@dependents]);
            1;
        } or do {
            print STDERR "error with $plugin->buildFinished: $@\n";
        }
    }
    # We have to iterate through all dependents as well, and if they are finished
    # to mark their notificationpendingsince.
    # Otherwise, the dependent builds will remain with notificationpendingsince set
    # until hydra-notify is started, as buildFinished is never emitted for them.
    foreach my $b ($build, @dependents) {
        $b->update({ notificationpendingsince => undef }) if $b->finished;
    }
}

Replacement in src/script/hydra-notify at line 52 [6.2994]

B:BD[9.2466] → [9.2466:2566]

B:BD[9.2566] → [11.136:167]

    my $buildId = $build->id;
    print STDERR "sending notifications for build ${\$buildId}...\n";
    buildFinished($build->id);

[9.2466]

[9.2593]

    print STDERR "sending notifications for build $build->id...\n";
    my $event = Hydra::Event::BuildFinished->new($build->id);
    runPluginsForEvent($event);

Deletion in src/script/hydra-notify at line 68 [6.2994]

∅:D[7.2149] → [15.108:219]

B:BD[15.108] → [15.108:219]

        #print STDERR "got '$channelName' from $pid: $payload\n";
        my @payload = split /\t/, $payload;

Replacement in src/script/hydra-notify at line 70 [6.2994]

B:BD[15.235] → [4.741:828]

∅:D[4.828] → [16.451:585]

B:BD[15.286] → [16.451:585]

∅:D[16.585] → [15.334:439]

B:BD[15.334] → [15.334:439]

B:BD[15.439] → [11.168:233]

B:BD[15.755] → [15.755:769]

            if ($channelName eq "build_started" || $channelName eq "step_finished" ) {
                my $event = Hydra::Event::new_event($channelName, $message->{"payload"});
                runPluginsForEvent($event);
            } elsif ($channelName eq "build_finished") {
                my $buildId = int($payload[0]);
                buildFinished($buildId, @payload[1..$#payload]);
            }

[15.235]

[13.1202]

            my $event = Hydra::Event::new_event($channelName, $message->{"payload"});
            runPluginsForEvent($event);

File addition: BuildFinished.t (----------)

[16.587]

use strict;
use Setup;
my %ctx = test_init();
require Hydra::Schema;
require Hydra::Model::DB;
use Hydra::Event;
use Hydra::Event::BuildFinished;
use Test2::V0;
use Test2::Tools::Exception;
use Test2::Tools::Mock qw(mock_obj);
my $db = Hydra::Model::DB->new;
hydra_setup($db);
subtest "Parsing" => sub {
    like(
        dies { Hydra::Event::parse_payload("build_finished", "") },
        qr/at least one argument/,
        "empty payload"
    );
    like(
        dies { Hydra::Event::parse_payload("build_finished", "abc123") },
        qr/should be integers/,
        "build ID should be an integer"
    );
    like(
        dies { Hydra::Event::parse_payload("build_finished", "123\tabc123") },
        qr/should be integers/,
        "dependent ID should be an integer"
    );
    is(
        Hydra::Event::parse_payload("build_finished", "123"),
        Hydra::Event::BuildFinished->new(123, []),
        "no dependent builds"
    );
    is(
        Hydra::Event::parse_payload("build_finished", "123\t456"),
        Hydra::Event::BuildFinished->new(123, [456]),
        "one dependent build"
    );
    is(
        Hydra::Event::parse_payload("build_finished", "123\t456\t789\t012\t345"),
        Hydra::Event::BuildFinished->new(123, [456, 789, 12, 345]),
        "four dependent builds"
    );
};
my $project = $db->resultset('Projects')->create({name => "tests", displayname => "", owner => "root"});
my $jobset = createBaseJobset("basic", "basic.nix", $ctx{jobsdir});
ok(evalSucceeds($jobset),               "Evaluating jobs/basic.nix should exit with return code 0");
is(nrQueuedBuildsForJobset($jobset), 3, "Evaluating jobs/basic.nix should result in 3 builds");
subtest "load" => sub {
    my ($build, $dependent_a, $dependent_b) = $db->resultset('Builds')->search(
      { },
      { limit => 3 }
    )->all;
    my $event = Hydra::Event::BuildFinished->new($build->id, [$dependent_a->id, $dependent_b->id]);
    $event->load($db);
    is($event->{"build"}->id, $build->id, "The build record matches.");
    is($event->{"dependents"}[0]->id, $dependent_a->id, "The dependent_a record matches.");
    is($event->{"dependents"}[1]->id, $dependent_b->id, "The dependent_b record matches.");
    # Create a fake "plugin" with a buildFinished sub, the sub sets this
    # global passedBuild and passedDependents variables for verifying.
    my $passedBuild;
    my $passedDependents;
    my $plugin = {};
    my $mock = mock_obj $plugin => (
        add => [
            "buildFinished" => sub {
                my ($self, $build, $dependents) = @_;
                $passedBuild = $build;
                $passedDependents = $dependents;
            }
        ]
    );
    $event->execute($db, $plugin);
    is($passedBuild->id, $build->id, "The plugin's buildFinished hook is called with a matching build");
    is($passedDependents->[0]->id, $dependent_a->id, "The plugin's buildFinished hook is called with a matching dependent_a");
    is($passedDependents->[1]->id, $dependent_b->id, "The plugin's buildFinished hook is called with a matching dependent_b");
};
done_testing;

Deletion in t/Event.t at line 3 [5.2970]
B:BD[5.3001] → [5.3001:3034]
```
use Hydra::Event::BuildFinished;
```

Deletion in t/Event.t at line 12 [5.2970]

B:BD[17.463] → [17.463:466]

B:BD[5.3478] → [5.3478:3479]

B:BD[5.4960] → [5.4960:6009]

};
subtest "Payload type: build_finished" => sub {
    like(
        dies { Hydra::Event::parse_payload("build_finished", "") },
        qr/at least one argument/,
        "empty payload"
    );
    like(
        dies { Hydra::Event::parse_payload("build_finished", "abc123") },
        qr/should be integers/,
        "build ID should be an integer"
    );
    like(
        dies { Hydra::Event::parse_payload("build_finished", "123\tabc123") },
        qr/should be integers/,
        "dependent ID should be an integer"
    );
    is(
        Hydra::Event::parse_payload("build_finished", "123"),
        Hydra::Event::BuildFinished->new(123, []),
        "no dependent builds"
    );
    is(
        Hydra::Event::parse_payload("build_finished", "123\t456"),
        Hydra::Event::BuildFinished->new(123, [456]),
        "one dependent build"
    );
    is(
        Hydra::Event::parse_payload("build_finished", "123\t456\t789\t012\t345"),
        Hydra::Event::BuildFinished->new(123, [456, 789, 12, 345]),
        "four dependent builds"
    );

Deletion in t/Event.t at line 14 [5.2970]
B:BD[5.6013] → [5.6013:6014]