hootenanny-ui icon indicating copy to clipboard operation
hootenanny-ui copied to clipboard

Can't override options in the new version of Advanced Options

Open bwitham opened this issue 5 years ago • 5 comments

The new version of Advanced Options seems to override any setting put in the conflateAdvOps.json file and replace it with the value in ConfigOptions.asciidoc. Most of the time the two sets of values should be in sync, but for some options we want the core and UI values to differ.

bwitham avatar Jul 03 '19 12:07 bwitham

@bwitham after #1363, the conflationTypes.json needs to be updated to expose specific options to the 2x UI.

Options should be added as a k:v pair where k=option name as shown in ConfigOptions.asciidoc and v=the alias you want in the ui. See here for an example.

I also think you'll need to rebuild services after you edit the json

maxgrossman avatar Jul 08 '19 15:07 maxgrossman

I'm still confused. I don't see any way to make the values different between UI and core...I may just be missing it. I wanted to make poi.polygon.address.match.enabled true in the UI but false in core. When I look at this JSON for that option, I don't see anywhere to edit the actual default value:

"PoiPolygon": {
        "matcher": "hoot::PoiPolygonMatchCreator",
        "merger": "hoot::PoiPolygonMergerCreator",
        "members": {
            "poi.polygon.address.match.enabled": "Enable address matching",
            "poi.polygon.disable.same.source.conflation": "Disable same source conflation",
            "poi.polygon.disable.same.source.conflation.match.tag.key.prefix.only": "Match key prefix only if same source conflation disabled",
            "poi.polygon.match.distance.threshold" : "Match distance threshold",
            "poi.polygon.match.evidence.threshold": "Match evidence threshold",
            "poi.polygon.name.score.threshold": "Name score threshold",
            "poi.polygon.review.distance.threshold": "Review distance threshold",
            "poi.polygon.review.evidence.threshold": "Review evidence threshold",
            "poi.polygon.review.if.matched.types": "Tags used to flag reviews",
            "poi.polygon.source.tag.key": "Source key used when disabling same source conflation",
            "poi.polygon.type.score.threshold": "Type score threshold"
        }
    },

Making that option false in the core leads to it always being false in the UI.

bwitham avatar Jul 08 '19 16:07 bwitham

Ok, my misunderstanding. I better grasp your requirement now.

I think what you are 'missing' is something this new file is 'missing'. I guess an 'at this very moment fix' would be to edit the ConfigOptions.json (built from the ConfigOptions.asciidoc). 2x reads in defaults from there. We wouldn't want to update the .ascii doc since that is read in by core right?

As for a long term/better solutions, maybe we just need to make that members object more configurable.

"PoiPolygon": {
        "matcher": "hoot::PoiPolygonMatchCreator",
        "merger": "hoot::PoiPolygonMergerCreator",
        "members": { 
            "poi.polygon.address.match.enabled": { name: "Enable address matching", default: true }
       }
}

where the new default key is optional and lets you override the default.

maxgrossman avatar Jul 08 '19 18:07 maxgrossman

Ok, I misunderstood what we currently had implemented for options. So, what you describe as a new feature that allows for selectively changing default values in ConfigOptions.json to be different than those in ConfigOptions.asciidoc is what's needed. I changed this to be a low priority feature, rather than a bug, since I found workarounds for both situations I encountered last week where I needed defaults to be different between the two. Obviously the vast majority of the time we want the UI and core options to be perfectly in sync, but there will be the occasional situation where some default values need to differ between the two.

bwitham avatar Jul 09 '19 13:07 bwitham

I think the issue is we refactored the config options so that UI and core WOULD always be in sync re: default params and values, because in the past the ui/services and core configs had diverged and caused confusion about differences in behavior. :point_up: wrote this before seeing most recent comment

brianhatchl avatar Jul 09 '19 20:07 brianhatchl