refactor: remove run page json type hack

this hack tried to avoid sending unnecessary data to the frontend,
but it was more trouble than it's worth
feature/contexts
Oliver Giles 5 years ago
parent 39ca7e86cf
commit ba472711be

@ -278,16 +278,10 @@ kj::Promise<void> Http::startServer(kj::Timer& timer, kj::Own<kj::ConnectionRece
return server->listenHttp(*listener).attach(cleanupPeers(timer)).attach(kj::mv(listener)).attach(kj::mv(server)); return server->listenHttp(*listener).attach(cleanupPeers(timer)).attach(kj::mv(listener)).attach(kj::mv(server));
} }
void Http::notifyEvent(const char *type, const char *data, std::string job, uint run) void Http::notifyEvent(const char *data, std::string job)
{ {
for(EventPeer* c : eventPeers) { for(EventPeer* c : eventPeers) {
if(c->scope.wantsStatus(job, run) if(c->scope.wantsStatus(job)) {
// The run page also should know that another job has started
// (so maybe it can show a previously hidden "next" button).
// Hence this small hack:
// TODO obviate
|| (std::string(type)=="job_started" && c->scope.type == MonitorScope::Type::RUN && c->scope.job == job))
{
c->pendingOutput.push_back("data: " + std::string(data) + "\n\n"); c->pendingOutput.push_back("data: " + std::string(data) + "\n\n");
c->fulfiller->fulfill(); c->fulfiller->fulfill();
} }

@ -40,7 +40,7 @@ public:
kj::Promise<void> startServer(kj::Timer &timer, kj::Own<kj::ConnectionReceiver> &&listener); kj::Promise<void> startServer(kj::Timer &timer, kj::Own<kj::ConnectionReceiver> &&listener);
void notifyEvent(const char* type, const char* data, std::string job = nullptr, uint run = 0); void notifyEvent(const char* data, std::string job = nullptr);
void notifyLog(std::string job, uint run, std::string log_chunk, bool eot); void notifyLog(std::string job, uint run, std::string log_chunk, bool eot);
private: private:

@ -574,7 +574,7 @@ std::shared_ptr<Run> Laminar::queueJob(std::string name, ParamMap params) {
.startObject("data") .startObject("data")
.set("name", name) .set("name", name)
.EndObject(); .EndObject();
http->notifyEvent("job_queued", j.str(), name.c_str()); http->notifyEvent(j.str(), name.c_str());
assignNewJobs(); assignNewJobs();
return run; return run;
@ -666,7 +666,7 @@ bool Laminar::tryStartRun(std::shared_ptr<Run> run, int queueIndex) {
} }
j.EndArray(); j.EndArray();
j.EndObject(); j.EndObject();
http->notifyEvent("job_started", j.str(), run->name.c_str(), run->build); http->notifyEvent(j.str(), run->name.c_str());
return true; return true;
} }
} }
@ -755,7 +755,7 @@ void Laminar::runFinished(Run * r) {
populateArtifacts(j, r->name, r->build); populateArtifacts(j, r->name, r->build);
j.EndArray(); j.EndArray();
j.EndObject(); j.EndObject();
http->notifyEvent("job_completed", j.str(), r->name, r->build); http->notifyEvent(j.str(), r->name);
http->notifyLog(r->name, r->build, "", true); http->notifyLog(r->name, r->build, "", true);
// erase reference to run from activeJobs. Since runFinished is called in a // erase reference to run from activeJobs. Since runFinished is called in a
// lambda whose context contains a shared_ptr<Run>, the run won't be deleted // lambda whose context contains a shared_ptr<Run>, the run won't be deleted
@ -797,7 +797,7 @@ bool Laminar::handleBadgeRequest(std::string job, std::string &badge) {
db->stmt("SELECT result FROM builds WHERE name = ? ORDER BY number DESC LIMIT 1") db->stmt("SELECT result FROM builds WHERE name = ? ORDER BY number DESC LIMIT 1")
.bind(job) .bind(job)
.fetch<int>([&](int result){ .fetch<int>([&](int result){
rs = (RunState) result; rs = RunState(result);
}); });
if(rs == RunState::UNKNOWN) if(rs == RunState::UNKNOWN)
return false; return false;

@ -41,19 +41,20 @@ struct MonitorScope {
order_desc(true) order_desc(true)
{} {}
// whether this scope wants status information about the given job or run // whether this scope wants status information for the specified job
bool wantsStatus(std::string ajob, uint anum = 0) const { bool wantsStatus(std::string ajob, uint anum = 0) const {
if(type == HOME || type == ALL) return true; if(type == HOME || type == ALL) return true;
if(type == JOB) return ajob == job; else return ajob == job;
if(type == RUN) return ajob == job && anum == num; // we could have checked that the run number matches, but actually the
return false; // run page needs to know about a non-matching run number in order to
// know whether to display the "next" arrow.
} }
Type type; Type type;
std::string job; std::string job;
uint num = 0; uint num ;
// sorting // sorting
uint page = 0; uint page;
std::string field; std::string field;
bool order_desc; bool order_desc;
}; };

@ -73,3 +73,31 @@ TEST_F(LaminarFixture, JobNotifyHomePage) {
EXPECT_STREQ("foo", job_completed["data"]["name"].GetString()); EXPECT_STREQ("foo", job_completed["data"]["name"].GetString());
} }
TEST_F(LaminarFixture, OnlyRelevantNotifications) {
defineJob("job1", "true");
defineJob("job2", "true");
auto esHome = eventSource("/");
auto esJobs = eventSource("/jobs");
auto es1Job = eventSource("/jobs/job1");
auto es2Job = eventSource("/jobs/job2");
auto es1Run = eventSource("/jobs/job1/1");
auto es2Run = eventSource("/jobs/job2/1");
auto req1 = client().runRequest();
req1.setJobName("job1");
ASSERT_EQ(LaminarCi::JobResult::SUCCESS, req1.send().wait(ioContext.waitScope).getResult());
auto req2 = client().runRequest();
req2.setJobName("job2");
ASSERT_EQ(LaminarCi::JobResult::SUCCESS, req2.send().wait(ioContext.waitScope).getResult());
ioContext.waitScope.poll();
EXPECT_EQ(7, esHome->messages().size());
EXPECT_EQ(7, esJobs->messages().size());
EXPECT_EQ(4, es1Job->messages().size());
EXPECT_EQ(4, es2Job->messages().size());
EXPECT_EQ(4, es1Run->messages().size());
EXPECT_EQ(4, es2Run->messages().size());
}

Loading…
Cancel
Save