-
Notifications
You must be signed in to change notification settings - Fork 1
Mig/delete old scenariorun prod 9734 #346
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
Conversation
vcarluer
left a comment
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.
reuse @LeopoldCramer method
scenariorun/src/main/kotlin/com/cosmotech/scenariorun/azure/ScenarioRunServiceImpl.kt
Outdated
Show resolved
Hide resolved
scenariorun/src/main/kotlin/com/cosmotech/scenariorun/azure/ScenarioRunServiceImpl.kt
Outdated
Show resolved
Hide resolved
jreynard-code
left a comment
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.
Some changes ;)
scenariorun/src/main/kotlin/com/cosmotech/scenariorun/azure/ScenarioRunServiceImpl.kt
Show resolved
Hide resolved
| return scenarioRun.withoutSensitiveData()!! | ||
| } | ||
|
|
||
| private fun deletePreviousSimulationDataIfCurrentSimulationIsSuccessful(currentRun: ScenarioRun) { |
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.
Please do not write while(true) it's awful.
try something like this
private fun deletePreviousSimulationDataIfCurrentSimulationIsSuccessful(currentRun: ScenarioRun) {
val scenarioRunId = currentRun.id!!
val workspaceId = currentRun.workspaceId!!
val organizationId = currentRun.organizationId!!
val scenarioId = currentRun.scenarioId!!
val csmSimulationRun = currentRun.csmSimulationRun
var scenarioRunStatus = getScenarioRunStatus(organizationId, scenarioRunId)
while (
scenarioRunStatus.state != ScenarioRunState.Successful ||
scenarioRunStatus.state != ScenarioRunState.Failed ||
scenarioRunStatus.state != ScenarioRunState.DataIngestionFailure ) {
logger.info("ScenarioRun {} is still running, waiting for purging data", csmSimulationRun)
Thread.sleep(10000)
scenarioRunStatus = getScenarioRunStatus(organizationId, scenarioRunId)
}
if (scenarioRunStatus.state == ScenarioRunState.Successful) {
logger.info("ScenarioRun {} is Successfull => purging data", csmSimulationRun)
deleteScenarioRunsByScenario(organizationId, workspaceId, scenarioId)
} else {
logger.info("ScenarioRun {} is in error {} => no purging data",
csmSimulationRun,
scenarioRunStatus.state)
}
}
Note that the value 10000 should be configurable
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.
It's a sore in the eye
with the following code the intention is clear and neat
private fun deletePreviousSimulationDataIfCurrentSimulationIsSuccessful(currentRun: ScenarioRun) {
while (true) {
Thread.sleep(10000)
val scenarioRunStatus = getScenarioRunStatus(currentRun.organizationId!!, currentRun.id!!)
if (scenarioRunStatus.state == ScenarioRunState.Successful) {
deleteScenarioRunsByScenario(
currentRun.organizationId!!, currentRun.workspaceId!!, currentRun.scenarioId!!)
return
} else if (scenarioRunStatus.state == ScenarioRunState.DataIngestionFailure ||
scenarioRunStatus.state == ScenarioRunState.Failed) {
logger.info("ScenarioRun {} failed", currentRun.csmSimulationRun)
break
}
}
}
scenariorun/src/main/kotlin/com/cosmotech/scenariorun/azure/ScenarioRunServiceImpl.kt
Show resolved
Hide resolved
ff4e5c4 to
27df9f6
Compare
…ts if the current run is successful
deletion of scenariorun by scenario used data from argo, but those can be lost, instead data from cosmosdb are used
…tate + state flag without containers env vars
948f67d to
24809a4
Compare
jreynard-code
left a comment
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
No description provided.