aboutsummaryrefslogtreecommitdiff
path: root/mod
diff options
context:
space:
mode:
Diffstat (limited to 'mod')
-rw-r--r--mod/mod-ci-github.cxx100
1 files changed, 72 insertions, 28 deletions
diff --git a/mod/mod-ci-github.cxx b/mod/mod-ci-github.cxx
index 2e2d160..b402af6 100644
--- a/mod/mod-ci-github.cxx
+++ b/mod/mod-ci-github.cxx
@@ -333,6 +333,12 @@ namespace brep
string build_id; // Full build id.
optional<string> node_id; // GitHub id.
optional<build_state> state;
+
+ string
+ state_string () const
+ {
+ return state ? to_string (*state) : "null";
+ }
};
vector<check_run> check_runs;
@@ -363,6 +369,9 @@ namespace brep
json () const;
};
+ ostream&
+ operator<< (ostream&, const service_data::check_run&);
+
bool ci_github::
handle_check_suite_request (check_suite_event cs)
{
@@ -930,14 +939,6 @@ namespace brep
//
for (const build& b: builds)
{
- // Include this build if this is a legitimate transition back to queued
- // from another state or it doesn't already have a stored check run.
- // Note: never go back on the built state.
- //
- string bid (check_run_name (b)); // Full Build ID.
-
- const service_data::check_run* cr (sd.find_check_run (bid));
-
// To keep things simple we are going to queue/create a new check run
// only if we have no corresponding state (which means we haven't yet
// done anything about this check run).
@@ -947,22 +948,31 @@ namespace brep
// building, which is probably not a big deal. Also, this sidesteps
// various "absent state" corner.
//
- if (cr == nullptr)
- {
- if (cr != nullptr)
- crs.emplace_back (move (*cr)).state = nullopt;
- else
- crs.emplace_back (move (bid), nullopt, nullopt);
+ // Note: never go back on the built state.
+ //
+ string bid (check_run_name (b)); // Full Build ID.
+ const service_data::check_run* scr (sd.find_check_run (bid));
+
+ if (scr == nullptr)
+ {
+ crs.emplace_back (move (bid), nullopt, nullopt);
bs.push_back (b);
}
- else if (!cr->state)
+ else if (!scr->state)
; // Ignore network issue.
else if (istate && *istate == build_state::building)
; // Ignore interrupted.
else
{
- // @@ TODO warn with some information
+ // Out of order queued notification or a rebuild (not allowed).
+ //
+ warn << *scr << ": "
+ << "unexpected transition from "
+ << (istate ? to_string (*istate) : "null") << " to "
+ << to_string (build_state::queued)
+ << "; previously recorded check_run state: "
+ << scr->state_string ();
}
}
@@ -992,7 +1002,6 @@ namespace brep
new_iat = obtain_installation_access_token (sd.installation_id,
move (*jwt),
error);
-
if (new_iat)
iat = &*new_iat;
}
@@ -1084,7 +1093,9 @@ namespace brep
return [bs = move (bs),
iat = move (new_iat),
- crs = move (crs)] (const tenant_service& ts) -> optional<string>
+ crs = move (crs),
+ error = move (error),
+ warn = move (warn)] (const tenant_service& ts) -> optional<string>
{
// NOTE: this lambda may be called repeatedly (e.g., due to transaction
// being aborted) and so should not move out of its captures.
@@ -1096,8 +1107,7 @@ namespace brep
}
catch (const invalid_argument& e)
{
- // @@ TODO: try to move error into this lambda?
- // error << "failed to parse service data: " << e;
+ error << "failed to parse service data: " << e;
return nullopt;
}
@@ -1118,8 +1128,9 @@ namespace brep
//
if (service_data::check_run* scr = sd.find_check_run (cr.build_id))
{
- // @@ TODO: let's warn about this out of order case to see how
- // often we get it.
+ warn << cr << " state " << scr->state_string ()
+ << " was stored before notified state " << cr.state_string ()
+ << " could be stored";
}
else
sd.check_runs.push_back (cr);
@@ -1327,10 +1338,23 @@ namespace brep
p.next_expect_member_array ("check_runs");
while (p.next_expect (event::begin_object, event::end_array))
{
- check_runs.emplace_back (
- p.next_expect_member_string ("build_id"),
- p.next_expect_member_string ("node_id"),
- to_build_state (p.next_expect_member_string ("state")));
+ string bid (p.next_expect_member_string ("build_id"));
+
+ optional<string> nid;
+ {
+ string* v (p.next_expect_member_string_null ("node_id"));
+ if (v != nullptr)
+ nid = *v;
+ }
+
+ optional<build_state> s;
+ {
+ string* v (p.next_expect_member_string_null ("state"));
+ if (v != nullptr)
+ s = to_build_state (*v);
+ }
+
+ check_runs.emplace_back (move (bid), move (nid), s);
p.next_expect (event::end_object);
}
@@ -1377,8 +1401,19 @@ namespace brep
{
s.begin_object ();
s.member ("build_id", cr.build_id);
- s.member ("node_id", *cr.node_id);
- s.member ("state", to_string (*cr.state));
+
+ s.member_name ("node_id");
+ if (cr.node_id)
+ s.value (*cr.node_id);
+ else
+ s.value (nullptr);
+
+ s.member_name ("state");
+ if (cr.state)
+ s.value (to_string (*cr.state));
+ else
+ s.value (nullptr);
+
s.end_object ();
}
s.end_array ();
@@ -1399,6 +1434,15 @@ namespace brep
return nullptr;
}
+ ostream&
+ operator<< (ostream& os, const service_data::check_run& cr)
+ {
+ os << "check_run { node_id: " << cr.node_id.value_or ("null")
+ << ", build_id: " << cr.build_id << " }";
+
+ return os;
+ }
+
// The rest is GitHub request/response type parsing and printing.
//