Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

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

Open
bwitham opened this issue Jul 3, 2019 · 5 comments
Open

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

bwitham opened this issue Jul 3, 2019 · 5 comments

Comments

@bwitham
Copy link

bwitham commented Jul 3, 2019

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.

@maxgrossman
Copy link
Contributor

@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

@bwitham
Copy link
Author

bwitham commented Jul 8, 2019

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.

@maxgrossman
Copy link
Contributor

maxgrossman commented Jul 8, 2019

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.

@bwitham
Copy link
Author

bwitham commented Jul 9, 2019

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.

@brianhatchl
Copy link
Contributor

brianhatchl commented Jul 9, 2019

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.
☝️ wrote this before seeing most recent comment

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

3 participants