GH-2753: Fuseki CLI arg customisation refactor #2773
Conversation
Refactors how the FusekiMain CLI can be customised to make it more flexible and interface driven. CLI extensions have to be statically registered prior to invoking FusekiMain but are now able to both customise the arguments available and to directly affect the FusekiServer.Builder without needing to sub-class FusekiMain
|
|
||
| // ---- Port | ||
| serverConfig.port = defaultPort; | ||
| builder.port(defaultPort); |
There was a problem hiding this comment.
- Don't set this default if a Jetty config file present
| @@ -56,7 +57,7 @@ | |||
| * Modules must not rely on a call to {@code serverStopped} happening.</li> | |||
| * </ul> | |||
| */ | |||
| public interface FusekiModule extends FusekiBuildCycle, FusekiStartStop, FusekiActionCycle { | |||
| public interface FusekiModule extends FusekiCliCustomiser, FusekiBuildCycle, FusekiStartStop, FusekiActionCycle { | |||
There was a problem hiding this comment.
- Remove this, doesn't make sense given the CLI lifecycle
There was a problem hiding this comment.
Maybe keep but return a record of three slots (DSG, label, update flag) rather than pass in ServerConfig.
There was a problem hiding this comment.
That code was only being called from FusekiMain so made more sense (to me at least) to inline it
| * | ||
| * @param fuseki Fuseki Main | ||
| */ | ||
| public default void customiseCli(FusekiMain fuseki) { |
There was a problem hiding this comment.
Better to return a list of ArgDecls (ArgModuleGeneral) to add? It reduces the exposed surface of FusekiMain.
| * @param fuseki Fuseki Main | ||
| * @param builder Fuseki Builder | ||
| */ | ||
| public default void processCliArgs(FusekiMain fuseki, FusekiServer.Builder builder) { |
There was a problem hiding this comment.
Is the argument checking going to be done by the cusomization? (e.g. if arg1 , then arg2 is not allowed)
Someway to pass in the args that have been set by the earlier call?
There was a problem hiding this comment.
If the server is "everythign" then one case is a customizer to influence themoduels actually run. i.e there might be args that are a "switch off" e.g.--nogeosparql -- an arg to not run a GeoSPARQL module.
There was a problem hiding this comment.
Is the argument checking going to be done by the cusomization? (e.g. if arg1 , then arg2 is not allowed)
Someway to pass in the args that have been set by the earlier call?
That was why I was passing FusekiMain in as you can call the various contains() and getValue() methods inherited from the rest of the Jena CLI machinery
There was a problem hiding this comment.
If the server is "everything" then one case is a customizer to influence the modules actually run. i.e there might be args that are a "switch off" e.g.
--nogeosparql-- an arg to not run a GeoSPARQL module.
Yeah I need to play around more with how modules interact and are configured here, it's tricky because modules aren't currently registered until after the CLI arguments are parsed which makes it somewhat circular as you can't have a module directly register its own arguments unless its explicitly registered as a customiser
There was a problem hiding this comment.
A simplifying assumption is that a set FusekiCliCustomiser and a set of modules.
builder.addCliCustomisers must be before the builder calls command line processing.
There is no need to add FusekiCliCustomiser to FusekiModule for all modules.
A module can still also have FusekiCliCustomiser.
There was a problem hiding this comment.
That was why I was passing
FusekiMainin as you can call the variouscontains()andgetValue()methods inherited from the rest of the Jena CLI machinery
That seems a good argument to changing FusekiMain so it does not extend CmdARQ, and instead be a FusekiMain with a command line object as a member.
I'd find it helpful to have a written description of the the flow is expected to be.
If we can make FusekiMain smaller with some clearly defined functionality in separate classes, then when there are several modules/customization they'll get what they are allowed to modifiy.
Refactors how the FusekiMain CLI can be customised to make it more flexible and interface driven. CLI extensions have to be statically registered prior to invoking FusekiMain but are now able to both customise the arguments available and to directly affect the FusekiServer.Builder without needing to sub-class FusekiMain
Currently some tests are broken so still a Work in Progress!
GitHub issue resolved #2753
Pull request Description:
By submitting this pull request, I acknowledge that I am making a contribution to the Apache Software Foundation under the terms and conditions of the Contributor's Agreement.
See the Apache Jena "Contributing" guide.