Improve PS API report-sink "replace" logic #315
Conversation
| } catch (e: Exception) { | ||
| logger.error { e.localizedMessage } | ||
| return null | ||
| /* private fun handleReportReplacement( |
There was a problem hiding this comment.
This appears to be added, but commented out. Can it be removed?
There was a problem hiding this comment.
TBD- as discussed.
| try { | ||
| logger.info("Removed all reports with stage name = $stageInfo?.stage") | ||
|
|
||
| /* |
There was a problem hiding this comment.
Can we remove this since added, but commented out?
There was a problem hiding this comment.
TBD- as discussed.
| @@ -47,9 +48,23 @@ class CouchbaseRepository( | |||
|
|
|||
| private val logger = KotlinLogging.logger {} | |||
|
|
|||
| /** | |||
There was a problem hiding this comment.
Do we need this comment about lazy creating the transactions? Seems like nothing is done below it and there is a similar comment a few lines below that roughly matches this one, along with the code to lazy create the transactions.
There was a problem hiding this comment.
TBD- as discussed.
| /** | ||
| * When we need transaction in the future we can use this function | ||
| */ | ||
| /* fun runTransaction(action: (TransactionAttemptContext) -> Unit) { |
There was a problem hiding this comment.
This new method is added but commented out. Do we need it?
There was a problem hiding this comment.
TBD- as discussed.
libs/commons-database/src/main/kotlin/gov/cdc/ocio/database/couchbase/CouchbaseCollection.kt
Outdated
Show resolved
Hide resolved
libs/commons-database/src/main/kotlin/gov/cdc/ocio/database/couchbase/CouchbaseCollection.kt
Outdated
Show resolved
Hide resolved
| @@ -64,6 +64,10 @@ dependencies { | |||
| implementation 'io.opentelemetry:opentelemetry-exporter-otlp:1.29.0' | |||
| implementation 'com.google.code.gson:gson:2.10.1' | |||
| implementation group: 'io.github.microutils', name: 'kotlin-logging-jvm', version: '3.0.5' | |||
| implementation("com.couchbase.client:java-client:3.4.2") | |||
There was a problem hiding this comment.
Do we need this added? If so, that's an issue - the report-sink should not have any implementation specific database stuff in it.
There was a problem hiding this comment.
I've removed it for now. But we may need it for "runTransaction" logic if the repository is CouchbaseRepository. Unless we pass the report and items to a generic method into CouchbaseRepository class in the commons-database. I guess that would be better.
pstatus-report-sink-ktor/src/main/kotlin/gov/cdc/ocio/processingstatusapi/ReportManager.kt
Show resolved
Hide resolved
| * @param ctx TransactionAttemptContext | ||
| * @param item T | ||
| */ | ||
| fun<T> upsertItem(ctx: TransactionAttemptContext,id: String, item: T) { |
There was a problem hiding this comment.
The code that would use this function is commented out. Do we need this?
There was a problem hiding this comment.
TBD- as discussed.
|
No description provided.