-
Notifications
You must be signed in to change notification settings - Fork 361
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
WX-1595 GCP Batch backend refactor to include the PAPI request manager #7412
Changes from 74 commits
ff8543c
5cca13a
0823351
0f5ab24
94d1aaf
af6b42d
fdb5c24
ff23056
ecc28d1
2afef28
a249835
7ef43da
71daebe
1e8bcef
ad51153
476a923
f92974b
0bf7a90
94ce5cd
76bb726
dbd6d24
e619ef0
057ee14
6fe1518
0412a9d
d71751d
3ccc34d
810a061
9acc240
7a4db8c
2db7ce0
62129c9
26603c8
29f3c4f
778c83a
7becc27
38eb051
64510cf
be118f5
b4a1b30
e8a61f1
d9c362a
11a02bb
1e2661b
290f065
50517e4
a333f65
bfd9dd1
54c152d
59c9675
83891cf
0cf2ba0
526447d
8d7c618
fa18a3c
f872fee
0e30684
c08b09b
23650c0
35dce53
cee36d9
6119047
f4511b9
b30be69
8f16dbc
28a62ef
0e4ddcb
f1a83ff
dca1420
eca754f
0095a87
837f86e
dcd0efe
3c9d020
e261cb9
1050850
75acee0
387e399
5f01bc9
37e5fde
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
This file was deleted.
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,57 @@ | ||
package cromwell.backend.google.batch.actors | ||
|
||
import akka.actor.{Actor, ActorLogging, ActorRef} | ||
import com.google.cloud.batch.v1.JobName | ||
import cromwell.backend.google.batch.api.BatchApiRequestManager.{BatchApiStatusQueryFailed, BatchStatusPollRequest} | ||
import cromwell.backend.google.batch.api.GcpBatchRequestFactory | ||
import cromwell.backend.google.batch.models.RunStatus | ||
import cromwell.backend.google.batch.monitoring.BatchInstrumentation | ||
import cromwell.backend.standard.StandardAsyncJob | ||
import cromwell.core.WorkflowId | ||
|
||
import scala.concurrent.{Future, Promise} | ||
import scala.util.{Failure, Success, Try} | ||
|
||
/** | ||
* Allows fetching a job status | ||
*/ | ||
trait BatchApiStatusRequestClient { this: Actor with ActorLogging with BatchInstrumentation => | ||
|
||
private var pollingActorClientPromise: Option[Promise[RunStatus]] = None | ||
|
||
def pollingActorClientReceive: Actor.Receive = { | ||
case status: RunStatus => | ||
log.debug(s"Polled status received: $status") | ||
pollSuccess() | ||
completePromise(Success(status)) | ||
case BatchApiStatusQueryFailed(query, e) => | ||
log.error(e, s"Poll status failed for job ${query.jobId}: ${e.getMessage}") | ||
completePromise(Failure(e)) | ||
Check warning on line 29 in supportedBackends/google/batch/src/main/scala/cromwell/backend/google/batch/actors/BatchApiStatusRequestClient.scala Codecov / codecov/patchsupportedBackends/google/batch/src/main/scala/cromwell/backend/google/batch/actors/BatchApiStatusRequestClient.scala#L27-L29
|
||
} | ||
|
||
private def completePromise(result: Try[RunStatus]): Unit = { | ||
pollingActorClientPromise foreach { _.complete(result) } | ||
pollingActorClientPromise = None | ||
} | ||
|
||
def pollStatus( | ||
workflowId: WorkflowId, | ||
jobName: JobName, | ||
backendSingletonActor: ActorRef, | ||
requestFactory: GcpBatchRequestFactory | ||
): Future[RunStatus] = | ||
pollingActorClientPromise match { | ||
case Some(p) => p.future | ||
Check warning on line 44 in supportedBackends/google/batch/src/main/scala/cromwell/backend/google/batch/actors/BatchApiStatusRequestClient.scala Codecov / codecov/patchsupportedBackends/google/batch/src/main/scala/cromwell/backend/google/batch/actors/BatchApiStatusRequestClient.scala#L44
|
||
case None => | ||
backendSingletonActor ! BatchStatusPollRequest( | ||
workflowId, | ||
self, | ||
requestFactory.queryRequest(jobName), | ||
StandardAsyncJob(jobName.toString) | ||
) | ||
|
||
val newPromise = Promise[RunStatus]() | ||
pollingActorClientPromise = Option(newPromise) | ||
newPromise.future | ||
} | ||
} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I have no problem merging as-is with these logs in place, seeing as we'll probably have some more rounds of debugging. That said, we'll probably want to reduce the
info
ones eventually.There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
For the time being, I have marked the noisy logs as debug and kept the rest as info, it will be help to debug some of the currently open issues.