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
refactor(triplestore): ZIO-fying triplestore service (DSP-904) #2059
refactor(triplestore): ZIO-fying triplestore service (DSP-904) #2059
Conversation
This reverts commit e723107.
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.
LGTM although I have to admint that I couldn't review this PR properly because of the amount of changed files.
@@ -334,12 +337,15 @@ object TriplestoreInternalException { | |||
* @param message a description of the error. | |||
* @param cause the original exception representing the cause of the error, if any. | |||
*/ | |||
case class TriplestoreResponseException(message: String, cause: Option[Throwable] = None) | |||
final case class TriplestoreResponseException(message: String, cause: Option[Throwable] = None) |
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.
Setting the case class to final
here makes sense. Shall we do it for all Error case classes?
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 those that we don't want to have any subclasses, we definitely should. Btw. This error is one of those that should be moved out of the shared project and to the triplestore service, as it is specific to it.
webapi/src/main/scala/org/knora/webapi/http/handler/KnoraExceptionHandler.scala
Outdated
Show resolved
Hide resolved
webapi/src/main/scala/org/knora/webapi/http/handler/KnoraExceptionHandler.scala
Outdated
Show resolved
Hide resolved
appActor | ||
.ask(GetAppState()) | ||
.mapTo[AppState] | ||
.fallbackTo(FastFuture.successful(AppStates.Stopped)) |
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.
What does this fallback do? Does it mean if there is no state found, the state is set to Stopped
?
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.
yes, something like that. Basically, this should never fail. There is a short amount of time during startup, where the appactor is not ready to answer these queries and this is where this would "fix" this case. I ran into it only in tests.
@@ -12,8 +12,7 @@ import io.swagger.annotations._ | |||
import org.knora.webapi.IRI |
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.
Do we still need this route?
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.
yes, we do. It should be refactored / renamed at some point.
...rc/test/scala/org/knora/webapi/responders/v2/ontology/DeleteCardinalitiesFromClassSpec.scala
Outdated
Show resolved
Hide resolved
webapi/src/test/scala/org/knora/webapi/store/triplestore/TriplestoreServiceManagerSpec.scala
Show resolved
Hide resolved
…tionHandler.scala Co-authored-by: irinaschubert <irina.schubert@dasch.swiss>
…tionHandler.scala Co-authored-by: irinaschubert <irina.schubert@dasch.swiss>
…ithub.com:dasch-swiss/dsp-api into wip/DEV-904-dsp-api-zio-fying-triplestore-service
Kudos, SonarCloud Quality Gate passed!
|
PR Checklist
Please check if your PR fulfills the following requirements:
PR Type
What kind of change does this PR introduce?
What is the current behavior?
Issue Number: DSP-904
What is the new behavior?
Does this PR introduce a breaking change?
Other information