powsybl-core icon indicating copy to clipboard operation
powsybl-core copied to clipboard

Fix LoadFlowResult status for retro-compatibility and replace uses of deprecated isOk

Open rolnico opened this issue 3 months ago • 2 comments

Please check if the PR fulfills these requirements

  • [x] The commit message follows our guidelines
  • [ ] Tests for the changes have been added (for bug fixes / features)
  • [x] Docs have been added / updated (for bug fixes / features)

Does this PR already have an issue describing the problem? No

What kind of change does this PR introduce? Bug fix

What is the current behavior? When initializing a new LoadFlowResultImpl(boolean ok, Map<String, String> metrics, String logs) with ok set to true, since no componentResults is given, the status is still set to FAILED. Therefore, there is an incoherence between isOk and status. This case appears for example when deserializing an old loadflow result where no component result is defined.

What is the new behavior (if this is a feature change)? If no component result is given during the initialisation of a new LoadFlowResultImpl and if ok is set to true, then status is set to DEFAULT_OK_STATUS_FOR_RETROCOMPATIBILITY, which has been defined equals to PARTIALLY_CONVERGED.

Does this PR introduce a breaking change or deprecate an API?

  • [ ] Yes
  • [x] No

If yes, please check if the following requirements are fulfilled

  • [ ] The Breaking Change or Deprecated label has been added
  • [ ] The migration steps are described in the following section

What changes might users need to make in their application due to this PR? (migration steps)

Other information:

rolnico avatar Apr 08 '24 08:04 rolnico

Quality Gate Failed Quality Gate failed

Failed conditions
59.5% Coverage on New Code (required ≥ 80%)

See analysis details on SonarCloud

sonarcloud[bot] avatar Apr 08 '24 08:04 sonarcloud[bot]

Hi, Wondering if we should now drop the LoadFlowResult isOk() support ... This decision was not taken in #2767, reason being that this means dropping also LoadFlowResult v1.0 support, but we may now reconsider this... Any thoughts ?

jeandemanged avatar Apr 30 '24 11:04 jeandemanged