CgmesImporter, allow importing with wrong dataextension when the mainfile doesn't exist #3147
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
…file doesn't exist This happens when filenames have the 2 following properties: - the filename contains a dot: - the filename does not contain the dataextension For example this filename triggers the bug: "microgrid.v2.complete.zip" but these work: "microgrid_v2_complete.zip" (because empty dataextension) or "microgrid.v2.complete.xml.zip" (because ".xml" dataextension) The problem is that the main extension is derived as ".v2.complete" but the cgmes importer only accepts an empty dataextension or ".xml" (it checks the dataextension to avoid importing as cgmes from a datasource that was meant by the user to be used as another format). But for now no other importer works without a main file, so when the mainfile doesn't exist so we shouldn't block the import just like when the dataextension is not specified NOTE: this happens because we want to allow archive filenames to be very flexible (with or without the dataextension, and for cgmes even with a totally different basename as what is inside). NOTE: if we ever have another importer that works like the cgmes importer, this means we will not be able to handle the following case: archive/network_EQ.xml archive/network_TP.xml # cgmes files archive/network_foo.other archive/network_bar.other # some new importer format and be able to import from a single directory containing both networks by specifying the dataextension. But this should be a rare case, and moving the data to separate directories or using separate basenames works around the problem. Signed-off-by: HARPER Jon <jon.harper87@gmail.com>
7fe3adc to
e196ede
Compare
Signed-off-by: HARPER Jon <jon.harper87@gmail.com>
d8abbc2 to
1d6a4e7
Compare
flo-dup
reviewed
Sep 19, 2024
cgmes/cgmes-model/src/test/java/com/powsybl/cgmes/model/CgmesOnDataSourceTest.java
Outdated
Show resolved
Hide resolved
cgmes/cgmes-model/src/test/java/com/powsybl/cgmes/model/CgmesOnDataSourceTest.java
Outdated
Show resolved
Hide resolved
Comment on lines
+68
to
+71
| Arguments.of("bar", "xml"), | ||
| Arguments.of("bar", null), | ||
| Arguments.of("bar", ""), | ||
| Arguments.of("bar", "notexists") |
There was a problem hiding this comment.
are they useful? I think each one corresponds to the same logic as one of the previous arguments.
Comment on lines
+120
to
+124
| Arguments.of("notexist", "xml", true), | ||
| Arguments.of("notexist", "xiidm", true), | ||
| Arguments.of("notexist", "notexists", true), | ||
| Arguments.of("notexist", "", true), | ||
| Arguments.of("notexist", null, true) |
…nDataSourceTest.java Co-authored-by: Florian Dupuy <66690739+flo-dup@users.noreply.github.com> Signed-off-by: Jon Harper <jon.harper87@gmail.com>
…nDataSourceTest.java Co-authored-by: Florian Dupuy <66690739+flo-dup@users.noreply.github.com> Signed-off-by: Jon Harper <jon.harper87@gmail.com>
Signed-off-by: HARPER Jon <jon.harper87@gmail.com>
af7cca2 to
2906e96
Compare
Signed-off-by: HARPER Jon <jon.harper87@gmail.com>
Signed-off-by: HARPER Jon <jon.harper87@gmail.com>
|
flo-dup
approved these changes
Sep 23, 2024
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
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?
can't import a cgmes zip with a dot in the name and not ".xml.zip" as the extension (e.g. microgrid.v2.complete.zip)
What is the new behavior (if this is a feature change)?
can import
Does this PR introduce a breaking change or deprecate an API?
Other information:
This happens when filenames have the 2 following properties:
"microgrid_v2_complete.zip" (because empty dataextension)
or "microgrid.v2.complete.xml.zip" (because ".xml" dataextension)
The problem is that the main extension is derived as ".complete" but the cgmes importer only accepts an empty dataextension or ".xml" (it checks the dataextension to avoid importing as cgmes from a datasource that was meant by the user to be used as another format). But for now no other importer works without a main file, so when the mainfile doesn't exist so we shouldn't block the import just like when the dataextension is not specified
NOTE: this happens because we want to allow archive filenames to be very flexible (with or without the dataextension, and for cgmes even with a totally different basename as what is inside).
NOTE: if we ever have another importer that works like the cgmes importer, this means we will not be able to handle the following case: archive/network_EQ.xml
archive/network_TP.xml # cgmes files
archive/network_foo.other
archive/network_bar.other # some new importer format and be able to import from a single directory containing both networks by specifying the dataextension. But this should be a rare case, and moving the data to separate directories or using separate basenames works around the problem.