Generalize extensions #23
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Signed-off-by: BOUHOURS Antoine <[email protected]>
antoinebhs
force-pushed
the
generalize-extension
branch
from
f945820 to
365c6e0
Compare
Signed-off-by: BOUHOURS Antoine <[email protected]>
antoinebhs
force-pushed
the
generalize-extension
branch
from
365c6e0 to
3b41004
Compare
1 task
…re-server into generalize-extension
Signed-off-by: BOUHOURS Antoine <[email protected]>
Signed-off-by: BOUHOURS Antoine <[email protected]>
antoinebhs
force-pushed
the
generalize-extension
branch
3 times, most recently
from
06afaf9 to
7abae19
Compare
Signed-off-by: BOUHOURS Antoine <[email protected]>
antoinebhs
force-pushed
the
generalize-extension
branch
from
7abae19 to
e544543
Compare
geofjamg
approved these changes
Apr 5, 2024
Signed-off-by: BOUHOURS Antoine <[email protected]>
FranckLecuyer
requested changes
Apr 8, 2024
| " = ? and " + VARIANT_NUM_COLUMN + " = ?"; | ||
| } | ||
|
|
||
| public static String buildExtensionsQuery(String columnNameForWhereClause) { |
There was a problem hiding this comment.
buildGetExtensionsQuery ?
There was a problem hiding this comment.
You're right, thanks!
Fixed in 99469ab.
| columnNameForWhereClause + " = ?"; | ||
| } | ||
|
|
||
| public static String buildExtensionsWithInClauseQuery(String columnNameForInClause, int numberOfValues) { |
There was a problem hiding this comment.
buildGetExtensionsWithInClauseQuery ?
There was a problem hiding this comment.
You're right, thanks!
Fixed in 99469ab.
Signed-off-by: BOUHOURS Antoine <[email protected]>
…re-server into generalize-extension
FranckLecuyer
approved these changes
Apr 9, 2024
Signed-off-by: BOUHOURS Antoine <[email protected]>
|
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Please check if the PR fulfills these requirements
What kind of change does this PR introduce?
It adds the ability to handle extensions discovered by service loader in the network store in a separate table "extension" instead of storing the extension with the identifiable. This introduces the possibility of not having the implementation of the extension in the network-store.
3 existing extensions have been migrated to this new framework: OperatingStatus, GeneratorStartup, ActivePowerControl.
The migration from the old framework to the new one is handled with SQL script in Liquibase.
This PR relies on powsybl/powsybl-network-store#383
Notes
→ Dump of dev-opf data => 7866159 extensions (OperatingStatus, GeneratorStartup or ActivePowerControl), migrated in 1 minute.
→ The data migration is only done for postgres but the schema migration for all dbms.
→ The migration does not drop the columns activePowerControl, operatingStatus and startup. This will be done in a future migration to be able to fallback on the previous implementation without data loss.
→ I haven't found a way to deserialize the json without adding the "extensionName" field when storing it (e.g.
{"extensionName":"activePowerControl","participate":true,"droop":0.0,"participationFactor":"NaN"}). The extensionName value is already stored in the "name" column so we could use it (?).→ I added a runtime dependencies to powsybl-network-store-iidm-impl to be able to discover ExtensionLoaders in this dependency at runtime.
→ On my computer, on a THT/HT study with 12000 extensions and ~6000 generators, we measure the time spent in NetworkStoreRepository methods only: