Skip to content

Commit fe82248

Browse files
fm3fm3
authored
Improve job list view, unify ID rendering (#9068)
### Description - Unified display of ids, allowing single-click copy. - Added link to workflow report to job list (superusers only for now). - Speed up loading of job list. - ~Fixed loading indicators for job and workflow lists.~ already done in #9095 - Refactored APIJob type in frontend and backend to make them simpler and make them match ### Steps to test: - `yarn enable jobs` - start a few jobs - check that workflow list view looks as expected - check that id rendering in job list view, annotation list view, etc is pretty. ### TODOs: - [x] Fix Loading indicator - [x] also on workflow list view (loading indicator in topright is there, but table still says No Data) - [x] Workflow link for superusers if available - [x] Fix rendering of infer instances job - [x] Speed up loading (old code ca 5s on scm orga) - [x] complex query in list view - [x] adapt types - [x] get single job json should be superset of compact list json - [x] test ### Issues: - fixes #9051 ------ - [x] Added changelog entry (create a `$PR_NUMBER.md` file in `unreleased_changes` or use `./tools/create-changelog-entry.py`) - [x] Removed dev-only changes like prints and application.conf edits - [x] Considered [common edge cases](../blob/master/.github/common_edge_cases.md)
1 parent fb5cc6b commit fe82248

39 files changed

+481
-400
lines changed

‎app/controllers/JobController.scala‎

Lines changed: 2 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -87,9 +87,8 @@ class JobController @Inject()(jobDAO: JobDAO,
8787
def list: Action[AnyContent] = sil.SecuredAction.async { implicit request =>
8888
for {
8989
_ <- Fox.fromBool(wkconf.Features.jobsEnabled) ?~> "job.disabled"
90-
jobs <- jobDAO.findAll
91-
jobsJsonList <- Fox.serialCombined(jobs.sortBy(_.created).reverse)(jobService.publicWrites)
92-
} yield Ok(Json.toJson(jobsJsonList))
90+
jobsCompact <- jobDAO.findAllCompact
91+
} yield Ok(Json.toJson(jobsCompact.map(_.enrich)))
9392
}
9493

9594
def get(id: ObjectId): Action[AnyContent] = sil.SecuredAction.async { implicit request =>

‎app/models/job/Job.scala‎

Lines changed: 97 additions & 64 deletions
Original file line numberDiff line numberDiff line change
@@ -6,7 +6,7 @@ import com.scalableminds.util.tools.{Fox, JsonHelper}
66
import com.scalableminds.webknossos.schema.Tables._
77
import models.job.JobState.JobState
88
import models.job.JobCommand.JobCommand
9-
import play.api.libs.json.{JsObject, Json}
9+
import play.api.libs.json.{JsObject, Json, OFormat}
1010
import slick.jdbc.PostgresProfile.api._
1111
import slick.jdbc.TransactionIsolation.Serializable
1212
import slick.lifted.Rep
@@ -22,19 +22,21 @@ case class Job(
2222
_owner: ObjectId,
2323
_dataStore: String,
2424
command: JobCommand,
25-
commandArgs: JsObject = Json.obj(),
25+
args: JsObject = Json.obj(),
2626
state: JobState = JobState.PENDING,
2727
manualState: Option[JobState] = None,
2828
_worker: Option[ObjectId] = None,
2929
_voxelyticsWorkflowHash: Option[String] = None,
3030
latestRunId: Option[String] = None,
3131
returnValue: Option[String] = None,
3232
retriedBySuperUser: Boolean = false,
33-
started: Option[Long] = None,
34-
ended: Option[Long] = None,
33+
started: Option[Instant] = None,
34+
ended: Option[Instant] = None,
3535
created: Instant = Instant.now,
3636
isDeleted: Boolean = false
37-
) {
37+
) extends JobResultLinks {
38+
protected def id: ObjectId = _id
39+
3840
def isEnded: Boolean = {
3941
val relevantState = manualState.getOrElse(state)
4042
relevantState == JobState.SUCCESS || state == JobState.FAILURE
@@ -44,58 +46,12 @@ case class Job(
4446
for {
4547
e <- ended
4648
s <- started
47-
} yield (e - s).millis
49+
} yield e - s
4850

49-
private def effectiveState: JobState = manualState.getOrElse(state)
51+
def effectiveState: JobState = manualState.getOrElse(state)
5052

5153
def exportFileName: Option[String] = argAsStringOpt("export_file_name")
5254

53-
def datasetName: Option[String] = argAsStringOpt("dataset_name")
54-
55-
def datasetId: Option[String] = argAsStringOpt("dataset_id")
56-
57-
private def argAsStringOpt(key: String) = (commandArgs \ key).toOption.flatMap(_.asOpt[String])
58-
private def argAsBooleanOpt(key: String) = (commandArgs \ key).toOption.flatMap(_.asOpt[Boolean])
59-
60-
def resultLink(organizationId: String): Option[String] =
61-
if (effectiveState != JobState.SUCCESS) None
62-
else {
63-
command match {
64-
case JobCommand.convert_to_wkw | JobCommand.compute_mesh_file =>
65-
datasetId.map { datasetId =>
66-
val datasetNameMaybe = datasetName.map(name => s"$name-").getOrElse("")
67-
Some(s"/datasets/$datasetNameMaybe$datasetId/view")
68-
}.getOrElse(datasetName.map(name => s"datasets/$organizationId/$name/view"))
69-
case JobCommand.export_tiff | JobCommand.render_animation =>
70-
Some(s"/api/jobs/${this._id}/export")
71-
case JobCommand.infer_neurons if this.argAsBooleanOpt("do_evaluation").getOrElse(false) =>
72-
returnValue.map { resultAnnotationLink =>
73-
resultAnnotationLink
74-
}
75-
case JobCommand.infer_nuclei | JobCommand.infer_neurons | JobCommand.materialize_volume_annotation |
76-
JobCommand.infer_with_model | JobCommand.infer_mitochondria | JobCommand.align_sections |
77-
JobCommand.infer_instances =>
78-
// There exist jobs with dataset name as return value, some with directoryName, and newest with datasetId
79-
// Construct links that work in either case.
80-
returnValue.map { datasetIdentifier =>
81-
ObjectId
82-
.fromStringSync(datasetIdentifier)
83-
.map { asObjectId =>
84-
s"/datasets/$asObjectId/view"
85-
}
86-
.getOrElse(s"/datasets/$organizationId/$datasetIdentifier/view")
87-
}
88-
case _ => None
89-
}
90-
}
91-
92-
def resultLinkPublic(organizationId: String, webknossosPublicUrl: String): Option[String] =
93-
for {
94-
resultLink <- resultLink(organizationId)
95-
resultLinkPublic = if (resultLink.startsWith("/")) s"$webknossosPublicUrl$resultLink"
96-
else s"$resultLink"
97-
} yield resultLinkPublic
98-
9955
def resultLinkSlackFormatted(organizationId: String, webknossosPublicUrl: String): String =
10056
(for {
10157
resultLink <- resultLinkPublic(organizationId, webknossosPublicUrl)
@@ -108,6 +64,37 @@ case class Job(
10864
}.getOrElse("")
10965
}
11066

67+
case class JobCompactInfo(
68+
id: ObjectId,
69+
command: JobCommand,
70+
organizationId: String,
71+
ownerFirstName: String,
72+
ownerLastName: String,
73+
ownerEmail: String,
74+
args: JsObject,
75+
state: JobState,
76+
returnValue: Option[String],
77+
resultLink: Option[String],
78+
voxelyticsWorkflowHash: Option[String],
79+
created: Instant,
80+
started: Option[Instant],
81+
ended: Option[Instant],
82+
creditCost: Option[scala.math.BigDecimal]
83+
) extends JobResultLinks {
84+
85+
protected def effectiveState: JobState = state
86+
87+
def enrich: JobCompactInfo = this.copy(
88+
resultLink = constructResultLink(organizationId),
89+
args = args - "webknossos_token" - "user_auth_token"
90+
)
91+
92+
}
93+
94+
object JobCompactInfo {
95+
implicit val jsonFormat: OFormat[JobCompactInfo] = Json.format[JobCompactInfo]
96+
}
97+
11198
class JobDAO @Inject()(sqlClient: SqlClient)(implicit ec: ExecutionContext)
11299
extends SQLDAO[Job, JobsRow, Jobs](sqlClient) {
113100
protected val collection = Jobs
@@ -135,8 +122,8 @@ class JobDAO @Inject()(sqlClient: SqlClient)(implicit ec: ExecutionContext)
135122
r.latestrunid,
136123
r.returnvalue,
137124
r.retriedbysuperuser,
138-
r.started.map(_.getTime),
139-
r.ended.map(_.getTime),
125+
r.started.map(Instant.fromSql),
126+
r.ended.map(Instant.fromSql),
140127
Instant.fromSql(r.created),
141128
r.isdeleted
142129
)
@@ -159,16 +146,62 @@ class JobDAO @Inject()(sqlClient: SqlClient)(implicit ec: ExecutionContext)
159146
)
160147
"""
161148

162-
private def listAccessQ(requestingUserId: ObjectId) =
163-
q"""_owner = $requestingUserId OR
164-
((SELECT u._organization FROM webknossos.users_ u WHERE u._id = _owner) IN (SELECT _organization FROM webknossos.users_ WHERE _id = $requestingUserId AND isAdmin))
165-
"""
149+
private def listAccessQ(requestingUserId: ObjectId, prefix: SqlToken) =
150+
q"""${prefix}_owner = $requestingUserId OR
151+
((SELECT u._organization FROM webknossos.users_ u WHERE u._id = ${prefix}_owner) IN (SELECT _organization FROM webknossos.users_ WHERE _id = $requestingUserId AND isAdmin))
152+
"""
166153

167-
override def findAll(implicit ctx: DBAccessContext): Fox[List[Job]] =
154+
def findAllCompact(implicit ctx: DBAccessContext): Fox[Seq[JobCompactInfo]] =
168155
for {
169-
accessQuery <- accessQueryFromAccessQ(listAccessQ)
170-
r <- run(q"SELECT $columns FROM $existingCollectionName WHERE $accessQuery ORDER BY created".as[JobsRow])
171-
parsed <- parseAll(r)
156+
accessQuery <- accessQueryFromAccessQWithPrefix(listAccessQ, q"j.")
157+
rows <- run(
158+
q"""
159+
SELECT j._id, j.command, u._organization, u.firstName, u.lastName, mu.email, j.commandArgs, COALESCE(j.manualState, j.state),
160+
j.returnValue, j._voxelytics_workflowHash, j.created, j.started, j.ended, ct.credit_delta
161+
FROM webknossos.jobs_ j
162+
JOIN webknossos.users_ u on j._owner = u._id
163+
JOIN webknossos.multiusers_ mu on u._multiUser = mu._id
164+
LEFT JOIN webknossos.credit_transactions_ ct ON j._id = ct._paid_job
165+
WHERE $accessQuery
166+
ORDER BY j.created DESC -- list newest first
167+
""".as[(ObjectId,
168+
String,
169+
String,
170+
String,
171+
String,
172+
String,
173+
String,
174+
String,
175+
Option[String],
176+
Option[String],
177+
Instant,
178+
Option[Instant],
179+
Option[Instant],
180+
Option[scala.math.BigDecimal])])
181+
parsed <- Fox.serialCombined(rows) { row =>
182+
for {
183+
command <- JobCommand.fromString(row._2).toFox
184+
effectiveState <- JobState.fromString(row._8).toFox
185+
commandArgs <- JsonHelper.parseAs[JsObject](row._7).toFox
186+
} yield
187+
JobCompactInfo(
188+
id = row._1,
189+
command = command,
190+
organizationId = row._3,
191+
ownerFirstName = row._4,
192+
ownerLastName = row._5,
193+
ownerEmail = row._6,
194+
args = commandArgs,
195+
state = effectiveState,
196+
returnValue = row._9,
197+
resultLink = None, // To be filled by calling “enrich”
198+
voxelyticsWorkflowHash = row._10,
199+
created = row._11,
200+
started = row._12,
201+
ended = row._13,
202+
creditCost = row._14.map(_ * -1) // delta is negative, so cost should be positive.
203+
)
204+
}
172205
} yield parsed
173206

174207
override def findOne(jobId: ObjectId)(implicit ctx: DBAccessContext): Fox[Job] =
@@ -250,7 +283,7 @@ class JobDAO @Inject()(sqlClient: SqlClient)(implicit ec: ExecutionContext)
250283
created, isDeleted
251284
)
252285
VALUES(
253-
${j._id}, ${j._owner}, ${j._dataStore}, ${j.command}, ${j.commandArgs},
286+
${j._id}, ${j._owner}, ${j._dataStore}, ${j.command}, ${j.args},
254287
${j.state}, ${j.manualState}, ${j._worker},
255288
${j.latestRunId}, ${j.returnValue}, ${j.started}, ${j.ended},
256289
${j.created}, ${j.isDeleted})""".asUpdate)

‎app/models/job/JobResultLinks.scala‎

Lines changed: 62 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,62 @@
1+
package models.job
2+
3+
import com.scalableminds.util.objectid.ObjectId
4+
import models.job.JobCommand.JobCommand
5+
import models.job.JobState.JobState
6+
import play.api.libs.json.JsObject
7+
8+
trait JobResultLinks {
9+
def args: JsObject
10+
def command: JobCommand
11+
def returnValue: Option[String]
12+
protected def id: ObjectId
13+
14+
protected def effectiveState: JobState
15+
16+
def datasetName: Option[String] = argAsStringOpt("dataset_name")
17+
18+
def datasetId: Option[String] = argAsStringOpt("dataset_id")
19+
20+
protected def argAsStringOpt(key: String): Option[String] = (args \ key).toOption.flatMap(_.asOpt[String])
21+
22+
protected def argAsBooleanOpt(key: String): Option[Boolean] = (args \ key).toOption.flatMap(_.asOpt[Boolean])
23+
24+
def constructResultLink(organizationId: String): Option[String] =
25+
if (effectiveState != JobState.SUCCESS) None
26+
else {
27+
command match {
28+
case JobCommand.convert_to_wkw | JobCommand.compute_mesh_file =>
29+
datasetId.map { datasetId =>
30+
val datasetNameMaybe = datasetName.map(name => s"$name-").getOrElse("")
31+
Some(s"/datasets/$datasetNameMaybe$datasetId/view")
32+
}.getOrElse(datasetName.map(name => s"/datasets/$organizationId/$name/view"))
33+
case JobCommand.export_tiff | JobCommand.render_animation =>
34+
Some(s"/api/jobs/${this.id}/export")
35+
case JobCommand.infer_neurons if this.argAsBooleanOpt("do_evaluation").getOrElse(false) =>
36+
returnValue.map { resultAnnotationLink =>
37+
resultAnnotationLink
38+
}
39+
case JobCommand.infer_nuclei | JobCommand.infer_neurons | JobCommand.materialize_volume_annotation |
40+
JobCommand.infer_with_model | JobCommand.infer_mitochondria | JobCommand.align_sections |
41+
JobCommand.infer_instances =>
42+
// There exist jobs with dataset name as return value, some with directoryName, and newest with datasetId
43+
// Construct links that work in either case.
44+
returnValue.map { datasetIdentifier =>
45+
ObjectId
46+
.fromStringSync(datasetIdentifier)
47+
.map { asObjectId =>
48+
s"/datasets/$asObjectId/view"
49+
}
50+
.getOrElse(s"/datasets/$organizationId/$datasetIdentifier/view")
51+
}
52+
case _ => None
53+
}
54+
}
55+
56+
def resultLinkPublic(organizationId: String, webknossosPublicUrl: String): Option[String] =
57+
for {
58+
resultLink <- constructResultLink(organizationId)
59+
resultLinkPublic = if (resultLink.startsWith("/")) s"$webknossosPublicUrl$resultLink"
60+
else s"$resultLink"
61+
} yield resultLinkPublic
62+
}

‎app/models/job/JobService.scala‎

Lines changed: 25 additions & 24 deletions
Original file line numberDiff line numberDiff line change
@@ -14,7 +14,7 @@ import models.organization.{CreditTransactionService, OrganizationDAO}
1414
import models.user.{MultiUserDAO, User, UserDAO, UserService}
1515
import com.scalableminds.util.tools.Full
1616
import org.apache.pekko.actor.ActorSystem
17-
import play.api.libs.json.{JsObject, Json}
17+
import play.api.libs.json.{JsObject, JsValue, Json}
1818
import security.WkSilhouetteEnvironment
1919
import telemetry.SlackNotificationService
2020
import utils.WkConf
@@ -158,7 +158,7 @@ class JobService @Inject()(wkConf: WkConf,
158158
if (job.state == JobState.FAILURE && job.command == JobCommand.convert_to_wkw) {
159159
logger.info(
160160
s"WKW conversion job ${job._id} failed. Deleting dataset from the database, freeing the directoryName...")
161-
val commandArgs = job.commandArgs.value
161+
val commandArgs = job.args.value
162162
for {
163163
datasetDirectoryName <- commandArgs.get("dataset_directory_name").map(_.as[String]).toFox
164164
organizationId <- commandArgs.get("organization_id").map(_.as[String]).toFox
@@ -168,31 +168,32 @@ class JobService @Inject()(wkConf: WkConf,
168168
} yield ()
169169
} else Fox.successful(())
170170

171-
def publicWrites(job: Job)(implicit ctx: DBAccessContext): Fox[JsObject] =
171+
def publicWrites(job: Job)(implicit ctx: DBAccessContext): Fox[JsValue] =
172172
for {
173173
owner <- userDAO.findOne(job._owner) ?~> "user.notFound"
174174
organization <- organizationDAO.findOne(owner._organization) ?~> "organization.notFound"
175-
resultLink = job.resultLink(organization._id)
176-
ownerJson <- userService.compactWrites(owner)
175+
ownerEmail <- userService.emailFor(owner)
177176
creditTransactionBox <- creditTransactionService.findTransactionOfJob(job._id).shiftBox
178-
} yield {
179-
Json.obj(
180-
"id" -> job._id.id,
181-
"owner" -> ownerJson,
182-
"command" -> job.command,
183-
"commandArgs" -> (job.commandArgs - "webknossos_token" - "user_auth_token"),
184-
"state" -> job.state,
185-
"manualState" -> job.manualState,
186-
"latestRunId" -> job.latestRunId,
187-
"returnValue" -> job.returnValue,
188-
"resultLink" -> resultLink,
189-
"voxelyticsWorkflowHash" -> job._voxelyticsWorkflowHash,
190-
"created" -> job.created,
191-
"started" -> job.started,
192-
"ended" -> job.ended,
193-
"creditCost" -> creditTransactionBox.toOption.map(t => (t.creditDelta * -1).toString)
177+
} yield
178+
Json.toJson(
179+
JobCompactInfo(
180+
id = job._id,
181+
command = job.command,
182+
organizationId = organization._id,
183+
ownerFirstName = owner.firstName,
184+
ownerLastName = owner.lastName,
185+
ownerEmail = ownerEmail,
186+
args = job.args - "webknossos_token" - "user_auth_token",
187+
state = job.effectiveState,
188+
returnValue = job.returnValue,
189+
resultLink = job.constructResultLink(organization._id),
190+
voxelyticsWorkflowHash = job._voxelyticsWorkflowHash,
191+
created = job.created,
192+
started = job.started,
193+
ended = job.ended,
194+
creditCost = creditTransactionBox.toOption.map(t => t.creditDelta * -1) // delta is negative, so cost should be positive.
195+
)
194196
)
195-
}
196197

197198
// Only seen by the workers
198199
def parameterWrites(job: Job)(implicit ctx: DBAccessContext): Fox[JsObject] =
@@ -204,7 +205,7 @@ class JobService @Inject()(wkConf: WkConf,
204205
Json.obj(
205206
"job_id" -> job._id.id,
206207
"command" -> job.command,
207-
"job_kwargs" -> (job.commandArgs ++ Json.obj("user_auth_token" -> userAuthToken.id))
208+
"job_kwargs" -> (job.args ++ Json.obj("user_auth_token" -> userAuthToken.id))
208209
)
209210
}
210211

@@ -222,7 +223,7 @@ class JobService @Inject()(wkConf: WkConf,
222223
jobBoundingBox: BoundingBox,
223224
creditTransactionComment: String,
224225
user: User,
225-
datastoreName: String)(implicit ctx: DBAccessContext): Fox[JsObject] =
226+
datastoreName: String)(implicit ctx: DBAccessContext): Fox[JsValue] =
226227
for {
227228
costsInCredits <- if (SHOULD_DEDUCE_CREDITS) calculateJobCostInCredits(jobBoundingBox, command)
228229
else Fox.successful(BigDecimal(0))

0 commit comments

Comments
 (0)