From 517dce285a851efd732affc084c7083aed2e98cd Mon Sep 17 00:00:00 2001 From: Graham Christensen Date: Mon, 7 Feb 2022 15:40:17 -0500 Subject: [PATCH] eval_added event: change interface to traceID\tjobsetID\tevaluationID I was not going to break the interface until I noticed the current implementation uses the string literal \t. --- doc/manual/src/notifications.md | 2 +- src/lib/Hydra/Event.pm | 2 + src/lib/Hydra/Event/EvalAdded.pm | 63 +++++++++++ src/lib/Hydra/Plugin.pm | 5 + src/script/hydra-eval-jobset | 2 +- src/script/hydra-notify | 1 + t/Hydra/Event/EvalAdded.t | 112 ++++++++++++++++++++ t/scripts/hydra-eval-jobset/notifications.t | 11 +- 8 files changed, 194 insertions(+), 4 deletions(-) create mode 100644 src/lib/Hydra/Event/EvalAdded.pm create mode 100644 t/Hydra/Event/EvalAdded.t diff --git a/doc/manual/src/notifications.md b/doc/manual/src/notifications.md index a2569172..f1203af8 100644 --- a/doc/manual/src/notifications.md +++ b/doc/manual/src/notifications.md @@ -63,7 +63,7 @@ It is possible for subsequent deliveries of the same `build_finished` data to im ### `eval_added` -* **Payload:** Exactly two values, separated by the two-character string `\t` (ie: not a tab): an opaque trace ID representing this evaluation, and the ID of the JobsetEval record. +* **Payload:** Exactly three values, tab separated: an opaque trace ID representing this evaluation, the ID of the jobset, and the ID of the JobsetEval record. * **When:** After the evaluator fetches inputs and completes the evaluation successfully. * **Delivery Semantics:** Ephemeral. `hydra-notify` must be running to react to this event. No record of this event is stored. diff --git a/src/lib/Hydra/Event.pm b/src/lib/Hydra/Event.pm index 0d50235e..9faf02e9 100644 --- a/src/lib/Hydra/Event.pm +++ b/src/lib/Hydra/Event.pm @@ -7,6 +7,7 @@ use Hydra::Event::BuildQueued; use Hydra::Event::BuildStarted; use Hydra::Event::CachedBuildFinished; use Hydra::Event::CachedBuildQueued; +use Hydra::Event::EvalAdded; use Hydra::Event::EvalCached; use Hydra::Event::EvalFailed; use Hydra::Event::EvalStarted; @@ -18,6 +19,7 @@ my %channels_to_events = ( build_started => \&Hydra::Event::BuildStarted::parse, cached_build_finished => \&Hydra::Event::CachedBuildFinished::parse, cached_build_queued => \&Hydra::Event::CachedBuildQueued::parse, + eval_added => \&Hydra::Event::EvalAdded::parse, eval_cached => \&Hydra::Event::EvalCached::parse, eval_failed => \&Hydra::Event::EvalFailed::parse, eval_started => \&Hydra::Event::EvalStarted::parse, diff --git a/src/lib/Hydra/Event/EvalAdded.pm b/src/lib/Hydra/Event/EvalAdded.pm new file mode 100644 index 00000000..fb3a4a8f --- /dev/null +++ b/src/lib/Hydra/Event/EvalAdded.pm @@ -0,0 +1,63 @@ +package Hydra::Event::EvalAdded; + +use strict; +use warnings; + +sub parse :prototype(@) { + unless (@_ == 3) { + die "eval_added: payload takes exactly three arguments, but ", scalar(@_), " were given"; + } + + my ($trace_id, $jobset_id, $evaluation_id) = @_; + + unless ($jobset_id =~ /^\d+$/) { + die "eval_added: payload argument jobset_id should be an integer, but '", $jobset_id, "' was given" + } + unless ($evaluation_id =~ /^\d+$/) { + die "eval_added: payload argument evaluation_id should be an integer, but '", $evaluation_id, "' was given" + } + + return Hydra::Event::EvalAdded->new($trace_id, int($jobset_id), int($evaluation_id)); +} + +sub new { + my ($self, $trace_id, $jobset_id, $evaluation_id) = @_; + return bless { + "trace_id" => $trace_id, + "jobset_id" => $jobset_id, + "evaluation_id" => $evaluation_id, + "jobset" => undef, + "evaluation" => undef + }, $self; +} + +sub interestedIn { + my ($self, $plugin) = @_; + return int(defined($plugin->can('evalAdded'))); +} + +sub load { + my ($self, $db) = @_; + + if (!defined($self->{"jobset"})) { + $self->{"jobset"} = $db->resultset('Jobsets')->find({ id => $self->{"jobset_id"}}) + or die "Jobset $self->{'jobset_id'} does not exist\n"; + } + + if (!defined($self->{"evaluation"})) { + $self->{"evaluation"} = $db->resultset('JobsetEvals')->find({ id => $self->{"evaluation_id"}}) + or die "Jobset $self->{'jobset_id'} does not exist\n"; + } +} + +sub execute { + my ($self, $db, $plugin) = @_; + + $self->load($db); + + $plugin->evalAdded($self->{"trace_id"}, $self->{"jobset"}, $self->{"evaluation"}); + + return 1; +} + +1; diff --git a/src/lib/Hydra/Plugin.pm b/src/lib/Hydra/Plugin.pm index ead65b9f..6cd46521 100644 --- a/src/lib/Hydra/Plugin.pm +++ b/src/lib/Hydra/Plugin.pm @@ -47,6 +47,11 @@ sub instantiate { # my ($self, $traceID, $jobset) = @_; # } +# # Called when $evaluation of $jobset has completed successfully. +# sub evalAdded { +# my ($self, $traceID, $jobset, $evaluation) = @_; +# } + # # Called when build $build has been queued. # sub buildQueued { # my ($self, $build) = @_; diff --git a/src/script/hydra-eval-jobset b/src/script/hydra-eval-jobset index 12ccfd5d..a68fe858 100755 --- a/src/script/hydra-eval-jobset +++ b/src/script/hydra-eval-jobset @@ -771,7 +771,7 @@ sub checkJobsetWrapped { }); $db->storage->dbh->do("notify eval_added, ?", undef, - join('\t', $tmpId, $ev->id)); + join("\t", $tmpId, $jobset->get_column('id'), $ev->id)); if ($jobsetChanged) { # Create JobsetEvalMembers mappings. diff --git a/src/script/hydra-notify b/src/script/hydra-notify index 40f87ffd..3b8ffe6d 100755 --- a/src/script/hydra-notify +++ b/src/script/hydra-notify @@ -98,6 +98,7 @@ $listener->subscribe("build_queued"); $listener->subscribe("build_started"); $listener->subscribe("cached_build_finished"); $listener->subscribe("cached_build_queued"); +$listener->subscribe("eval_added"); $listener->subscribe("eval_cached"); $listener->subscribe("eval_failed"); $listener->subscribe("eval_started"); diff --git a/t/Hydra/Event/EvalAdded.t b/t/Hydra/Event/EvalAdded.t new file mode 100644 index 00000000..140fa462 --- /dev/null +++ b/t/Hydra/Event/EvalAdded.t @@ -0,0 +1,112 @@ +use strict; +use warnings; +use Setup; +use Hydra::Event; +use Hydra::Event::EvalAdded; +use Test2::V0; +use Test2::Tools::Exception; +use Test2::Tools::Mock qw(mock_obj); + + +my $ctx = test_context(); +my $builds = $ctx->makeAndEvaluateJobset( + expression => "basic.nix", + build => 1 +); + + +subtest "Parsing eval_added" => sub { + like( + dies { Hydra::Event::parse_payload("eval_added", "") }, + qr/three arguments/, + "empty payload" + ); + like( + dies { Hydra::Event::parse_payload("eval_added", "abc123") }, + qr/three arguments/, + "one argument" + ); + like( + dies { Hydra::Event::parse_payload("eval_added", "abc123\tabc123") }, + qr/three arguments/, + "two arguments" + ); + like( + dies { Hydra::Event::parse_payload("eval_added", "abc123\tabc123\tabc123\tabc123") }, + qr/three arguments/, + "four arguments" + ); + like( + dies { Hydra::Event::parse_payload("eval_added", "abc123\tabc123\t123") }, + qr/should be an integer/, + "not an integer: second position" + ); + like( + dies { Hydra::Event::parse_payload("eval_added", "abc123\t123\tabc123") }, + qr/should be an integer/, + "not an integer: third position" + ); + is( + Hydra::Event::parse_payload("eval_added", "abc123\t123\t456"), + Hydra::Event::EvalAdded->new("abc123", 123, 456) + ); +}; + +subtest "interested" => sub { + my $event = Hydra::Event::EvalAdded->new("abc123", 123, 456); + + subtest "A plugin which does not implement the API" => sub { + my $plugin = {}; + my $mock = mock_obj $plugin => (); + + is($event->interestedIn($plugin), 0, "The plugin is not interesting."); + }; + + subtest "A plugin which does implement the API" => sub { + my $plugin = {}; + my $mock = mock_obj $plugin => ( + add => [ + "evalAdded" => sub {} + ] + ); + + is($event->interestedIn($plugin), 1, "The plugin is interesting."); + }; +}; + +subtest "load" => sub { + my $jobset = $builds->{"empty_dir"}->jobset; + my $evaluation = $builds->{"empty_dir"}->jobsetevals->first(); + + my $event = Hydra::Event::EvalAdded->new("traceID", $jobset->id, $evaluation->id); + + $event->load($ctx->db()); + is($event->{"trace_id"}, "traceID", "The Trace ID matches"); + is($event->{"jobset_id"}, $jobset->id, "The Jobset ID matches"); + is($event->{"evaluation_id"}, $evaluation->id, "The Evaluation ID matches"); + + + # Create a fake "plugin" with a evalAdded sub, the sub sets these + # "globals" + my $passedTraceID; + my $passedJobset; + my $passedEvaluation; + my $plugin = {}; + my $mock = mock_obj $plugin => ( + add => [ + "evalAdded" => sub { + my ($self, $traceID, $jobset, $evaluation) = @_; + $passedTraceID = $traceID; + $passedJobset = $jobset; + $passedEvaluation = $evaluation; + } + ] + ); + + $event->execute($ctx->db(), $plugin); + is($passedTraceID, "traceID", "We get the expected trace ID"); + is($passedJobset->id, $jobset->id, "The correct jobset is passed"); + is($passedEvaluation->id, $evaluation->id, "The correct evaluation is passed"); +}; + +done_testing; diff --git a/t/scripts/hydra-eval-jobset/notifications.t b/t/scripts/hydra-eval-jobset/notifications.t index e8c3281f..d2412432 100644 --- a/t/scripts/hydra-eval-jobset/notifications.t +++ b/t/scripts/hydra-eval-jobset/notifications.t @@ -67,13 +67,18 @@ subtest "on the initial evaluation" => sub { is($listener->block_for_messages(0)->()->{"channel"}, "build_queued", "expect 2/4 builds being queued"); is($listener->block_for_messages(0)->()->{"channel"}, "build_queued", "expect 3/4 builds being queued"); is($listener->block_for_messages(0)->()->{"channel"}, "build_queued", "expect 4/4 builds being queued"); - is($listener->block_for_messages(0)->()->{"channel"}, "eval_added", "the evaluation has completed"); + + expectEvent($listener, "eval_added", sub { + is($_->{"jobset_id"}, $jobset->get_column('id'), "the jobset ID matches"); + is($_->{"evaluation_id"}, $evaluation->get_column('id'), "the evaluation ID matches"); + }); is($listener->block_for_messages(0)->()->{"channel"}, "builds_added", "new builds have been scheduled"); is($listener->block_for_messages(0)->(), undef, "there are no more messages from the evaluator"); }; subtest "on a subsequent, totally cached / unchanged evaluation" => sub { ok(evalSucceeds($jobset), "evaluating for the second time"); + my $evaluation = $builds->{"stable-job-queued"}->jobsetevals->first(); my $traceID; expectEvent($listener, "eval_started", sub { @@ -103,7 +108,9 @@ subtest "on a fresh evaluation with changed sources" => sub { $builds->{"stable-job-failing"}->discard_changes(); ok(evalSucceeds($builds->{"variable-job"}->jobset), "evaluating for the third time"); - is($listener->block_for_messages(0)->()->{"channel"}, "eval_started", "the evaluation started"); + expectEvent($listener, "eval_started", sub { + is($_->{"jobset_id"}, $jobset->get_column('id'), "the jobset ID matches"); + }); # The order of builds is randomized when writing to the database, # so we can't expect the list in any specific order here.