Skip to content

Conversation

@michel-guillon
Copy link
Contributor

No description provided.

Copy link
Contributor

@vcarluer vcarluer left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Reviewed with @jreynard-code
Please retest after correction: deleting 1 scenariorun should not delete any scenario data

@michel-guillon michel-guillon force-pushed the delete_scenario_data_in_adx_Prod-9897 branch from 12c9451 to eea1749 Compare September 22, 2022 14:30
@vcarluer
Copy link
Contributor

IMHO: Squashing all commits we lose completely track of previous PR and what have been done for a feature

Copy link
Contributor

@vcarluer vcarluer left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

duplicate deletion call

organizationId, scenarioRun.workspaceKey!!, scenarioRun.csmSimulationRun!!)
}

private fun deleteScenarioRunMetadata(
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This method is not needed: ADX csmSimulation drop by extent already done in deleteScenarioRunWithoutAccessEnforcement line 135

}
this.deleteScenarioRunWithoutAccessEnforcement(scenarioRun)
deleteScenarioRunWithoutAccessEnforcement(scenarioRun)
deleteScenarioRunMetadata(
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This method call is not needed: ADX csmSimulation drop by extent already done in deleteScenarioRunWithoutAccessEnforcement line 135

Copy link
Contributor

@vcarluer vcarluer left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

@michel-guillon michel-guillon merged commit 7feab7e into main Sep 26, 2022
@jreynard-code jreynard-code deleted the delete_scenario_data_in_adx_Prod-9897 branch June 6, 2023 12:49
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants