refactor: app actor cleanup #2230
Conversation
|
Just some minor cleanup making the code (hopefully) simpler to read |
Codecov ReportBase: 86.64% // Head: 86.85% // Increases project coverage by
Additional details and impacted files@@ Coverage Diff @@
## main #2230 +/- ##
==========================================
+ Coverage 86.64% 86.85% +0.21%
==========================================
Files 233 241 +8
Lines 28020 27970 -50
==========================================
+ Hits 24277 24294 +17
+ Misses 3743 3676 -67
Help us with your feedback. Take ten seconds to tell us how you rate us. Have a feature suggestion? Share it here. ☔ View full report at Codecov. |
| for { | ||
| _ <- state.set(AppState.WaitingForIIIFService) | ||
| _ <- if (requiresIIIFService) | ||
| iiifs | ||
| .getStatus() | ||
| .flatMap(status => | ||
| status match { | ||
| case IIIFServiceStatusOK => | ||
| ZIO.logInfo("IIIF service running") | ||
| case IIIFServiceStatusNOK => | ||
| ZIO.logError("IIIF service not running") *> ZIO.die(new Exception("IIIF service not running")) | ||
| } | ||
| ) | ||
| else | ||
| ZIO.unit | ||
| _ <- iiifs | ||
| .getStatus() | ||
| .flatMap { | ||
| case IIIFServiceStatusOK => | ||
| ZIO.logInfo("IIIF service running") | ||
| case IIIFServiceStatusNOK => | ||
| ZIO.logError("IIIF service not running") *> ZIO.die(new Exception("IIIF service not running")) | ||
| } | ||
| .when(requiresIIIFService) | ||
| _ <- state.set(AppState.IIIFServiceReady) |
There was a problem hiding this comment.
I'm just wondering if it's actually reasonable to set the AppState to IIIF stuff, if we don't require the IIIF service. So maybe this method could be rewritten to:
private def checkIIIFService(requiresIIIFService: Boolean): ZIO[Any, Nothing, Unit] =
if (requiresIIIFService) {
for {
_ <- state.set(AppState.WaitingForIIIFService)
status <- iiifs.getStatus()
_ <- status match {
case CacheServiceStatusNOK =>
ZIO.logError("Cache service not running.") *>
ZIO.die(new Exception("Cache service not running."))
case CacheServiceStatusOK =>
ZIO.unit
}
_ <- state.set(AppState.IIIFServiceReady)
} yield ()
} else ZIO.unitoptionally, you could also use ZIO.when to test the bool condition. And even ZIO.whenCase to check the status
There was a problem hiding this comment.
yes, good point. With ZIO I'm actually not sure if we need these states in such a detail at all, or if something waiting for startup is not good enough. Now that we have logging that works, we should quickly figure out where the startup is stuck. So I will merge it as is, and we can create a follow-up issue after we discuss it.
they are now checked inside |
Issue Number: DEV-
Pull Request Checklist
Basic Requirements
Please check if your PR fulfills the following requirements:
PR Type
What kind of change does this PR introduce?
Does this PR introduce a breaking change?
Does this PR change client-test-data?
Other information