Enable config/command-line switch replacing embedded JAR resources with external files #1855
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
illuque-iov
force-pushed
the
resoucefile
branch
from
06c0eb3 to
e2121a7
Compare
illuque-iov
force-pushed
the
resoucefile
branch
from
e2121a7 to
e2976b0
Compare
illuque-iov
force-pushed
the
resoucefile
branch
from
e2976b0 to
652f530
Compare
|
Kudos, SonarCloud Quality Gate passed! |
|
pipeline:run |
6 similar comments
|
pipeline:run |
|
pipeline:run |
|
pipeline:run |
|
pipeline:run |
|
pipeline:run |
|
pipeline:run |
rmoreliovlabs
requested changes
Oct 7, 2022
| @@ -1386,7 +1389,9 @@ protected StateRootsStore buildStateRootsStore() { | |||
| protected synchronized RskSystemProperties buildRskSystemProperties() { | |||
| checkIfNotClosed(); | |||
|
|
|||
| return new RskSystemProperties(new ConfigLoader(cliArgs)); | |||
| RskSystemProperties props = new RskSystemProperties(new ConfigLoader(cliArgs)); | |||
There was a problem hiding this comment.
We should use getRskSystemProperties() instead?
| @@ -148,6 +149,8 @@ public class BridgeSupport { | |||
|
|
|||
| private static final String INVALID_ADDRESS_FORMAT_MESSAGE = "invalid address format"; | |||
|
|
|||
| private static final ConfigFileLoader.ResourceLoader RESOURCE_LOADER = BridgeSupport.class::getResourceAsStream; | |||
There was a problem hiding this comment.
Maybe rename this to BRIDGE_RESOURCE_LOADER?
|
Kudos, SonarCloud Quality Gate passed! |
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.
We provide a method to force rskj to load internal resource files from external files. This is controlled by configuration or command-line switches.
Description
We add a new section in the configuration file like this:
This sections can specify a folder (without filename) or a filename with path, depending on the key.
The supported path remappings are:
build-info(specifies a path to a folder, the file name is always "build-info.properties")remasc(the remasc configuration file)rskbitcoincheckpoints(the path to the folder containing a "*.checkpoints" file)genesis(the path to the folder containing a genesis file. The file name is taken from the config key "genesis")If a key is not given, no remapping is performed.
Motivation and Context
The benchmarking tool requires using different genesis files. To replace them, it's necessary to put the new genesis file somewhere in the class path and use a number of obscure methods to force Java to load it out of the JAR file. Many times it has failed. When it fails, it's impossible to diagnose the cause.
The channelling of all resource loads through a single method
loadConfigurationFile()has other benefits:remascfile is loaded each time when a block is processed (inefficient) and therskbitcoincheckpointsfile is not loaded on startup, but only when the certain bridge methods are called. The existence of both files should be checked at startup for sanity.How Has This Been Tested?
Tested was limited to:
-Xpath-remaps.genesis=/tmp/genesiscommand line argument and checking that it works as expected.path-remapssection.Types of changes
Checklist:
There is no documentation for none of the configuration options in the code, so no documentation was added to the code.
I will contact the developer's site maintainer to update the page https://developers.rsk.co/rsk/node/configure/reference/
Other information:
I suggest that after or concurrent with this merge, some tests are added to verify the correct placement of resource files.