Skip to content

Set the root report bundle resource to get the powsybl-core resource … #1207

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

Open
wants to merge 2 commits into
base: main
Choose a base branch
from

Conversation

alicecaron
Copy link
Contributor

@alicecaron alicecaron commented Mar 20, 2025

DO NOT MERGE. THIS PR IS SUPERSEDED BY #1216
(PR done perpared in case severe issue occure in 1.15.0)

Please check if the PR fulfills these requirements

  • The commit message follows our guidelines
  • Tests for the changes have been added (for bug fixes / features)
  • Docs have been added / updated (for bug fixes / features)
  • A PR or issue has been opened in all impacted repositories (if any)

What kind of change does this PR introduce?
Follow migration guide for core version 6.7.0-RC1 on ReportNode to load powsybl-core bundle for message templates.

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

  • Yes
  • No

@alicecaron alicecaron requested review from flo-dup and olperr1 March 20, 2025 14:09
@alicecaron alicecaron force-pushed the set-root-report-bundle branch from 7298ca3 to 38498ad Compare March 20, 2025 14:32
Copy link
Member

@olperr1 olperr1 left a comment

Choose a reason for hiding this comment

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

There is another usage of ReportNode.newRootReportNode(), in AbstractSecurityAnalysis at line 221.

@alicecaron alicecaron force-pushed the set-root-report-bundle branch from 38498ad to 657c293 Compare March 20, 2025 15:29
@alicecaron
Copy link
Contributor Author

There is another usage of ReportNode.newRootReportNode(), in AbstractSecurityAnalysis at line 221.

Yes! I corrected to precise the powsyl core bundle name to keep it light

@alicecaron alicecaron force-pushed the set-root-report-bundle branch from eaf1267 to 03956a9 Compare March 20, 2025 15:55
@alicecaron alicecaron requested a review from olperr1 March 20, 2025 15:55
@alicecaron alicecaron moved this from TODO to Waiting for review in Release 03/2025 Mar 20, 2025
@alicecaron alicecaron self-assigned this Mar 20, 2025
@alicecaron alicecaron changed the title Set the root report bundle resource to get the powsybl-core redource … Set the root report bundle resource to get the powsybl-core resource … Mar 26, 2025
@@ -623,6 +624,7 @@ public static ReportNode createLoadFlowReporter(ReportNode reportNode, String ne

public static ReportNode createRootLfNetworkReportNode(int networkNumCc, int networkNumSc) {
return ReportNode.newRootReportNode()
.withResourceBundles(PowsyblCoreReportResourceBundle.BASE_NAME)
.withMessageTemplate(LF_NETWORK_KEY, "Network CC${networkNumCc} SC${networkNumSc}")
Copy link
Collaborator

Choose a reason for hiding this comment

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

@jeandemanged how does this work for you ?
The bottom line is that:
- All messages moving forward, should be in a resource bundle (to enable multi-language support)
- If open load flow creates a root report node, all messages added to the report node should be in the root bundle list
- This includes:
- calls made to core (covered by this PR)
- messages added by OLF (not covered by this PR)
- calls made to OLF extensions

As of today, there is an alternative API currently to add all bundles in the class path to the report.

We create root reports today in mutlithreaded secu analysis, and in some other occasions, ofr example where we want to control the addition of a node depending whether it has children or note.
-

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants