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

[mapstore] change approach of tracking default config #240

Open
tkohr opened this issue May 24, 2022 · 8 comments
Open

[mapstore] change approach of tracking default config #240

tkohr opened this issue May 24, 2022 · 8 comments

Comments

@tkohr
Copy link
Collaborator

tkohr commented May 24, 2022

After some discussion and investigation (and browsing through issues from the past) I am wondering if we could not simplify the process of keeping up with default configurations for mapstore.

Of what I could identify, the main ups and downs of the currently "externalized" configs and patching the default configs shipped with the webapp are:

externalize patch
+ easier to read - harder to read
- difficult to update + easy to update
~ full control on deployed features ~ automatic deploy of new features

This makes me wonder if it would not make sense to switch from "externalizing" to "patching" in the georchestra datadir (at least for the two configs that are most impacted by new developments). This would allow staying automatically aligned with these developments.

To be more precise, I'm thinking of this:

  • remove localConfig.json and pluginsConfig.json from datadir
  • motivate and explain the use of patching these two configs (via localConfig.json.patch and pluginsConfig.json.patch) in the readme
  • mention the possibility to externalize these configs, but clarify what this means for updates in the readme (and point to the doc that already treats this)

I do not know the complete history of the mapstore datadir in georchestra and might be missing reasons, why certain files have been included. So please correct me.

Note: One potential drawback that I've noticed playing around with patching the localConfig.json is the technical limitation of targeting elements in an array other than by their position, which makes replacing plugins via a patch (without potentially breaking future configs) difficult for example.

Question: I saw that overriding is also a possibility, but had some trouble applying this in georchestra. Is it meant to work within mapstore2-georchestra, as well?

@fvanderbiest
Copy link
Member

Thanks for the investigation Tobias !

I would be +1 on the patching approach, but a bit worried about the technical limitation you mention.
cc @landryb @catmorales

@landryb
Copy link
Member

landryb commented May 30, 2022

i'm -1 on the approach. This has to stay simple, as merging configs in a visual diff editor is, writing patches for json is not something regular humans should do (i know, technically for developers it looks sexy and neat and everything, i know i will be able to manage them, but i wouldn't expect many people to)

Really, this should be discussed with lots of various platform admins dealing with the current config files... from the feedback gathered at geocom, people want a full config with examples, rather than cryptic json.patch files.

And please show us convicing examples :) Right now i saw nothing of what such patches would look like in our context (besides existing pluginsConfig.patch which is cheating since its the ms2 ui writing them)

@fvanderbiest
Copy link
Member

Agreed, but on the other hand, this would allow us to disconnect mapstore versions from geOrchestra versions... which is highly desirable IMO.

@fvanderbiest
Copy link
Member

If people want something easy to read, they have the possibility to commit in their own datadirs full versions of these files.

@tkohr
Copy link
Collaborator Author

tkohr commented May 30, 2022

I totally agree that patching is less readable than externalizing. I'm just thinking, this being the default datadir for georchestra it would simplify the workflow a lot to stay aligned with the default mapstore configs here. The admin will still be able to choose whether to externalize or patch the configuration of each deployment. While we could motivate patching, we could still point to https://github.com/georchestra/mapstore2-georchestra/tree/master/configs that can be used as a base for an externalized config.

Here's a small example patch to replace the default services listed for adding data:

[{
    "op": "replace",
    "path": "/initialState/defaultState/catalog/default/services",
    "value": {
      "example_streets": {
        "url": "https://www.example.fr/geoserver/ows",
        "type": "wms",
        "title": "Streets",
        "autoload": true
      },
      ...
    }
}]

One thing I stay concerned about is manipulating arrays via patches, which mainly concerns the configured plugins in the desktop, mobile and embedded sections. Replacing or removing a plugin via a fix index here, would risk to replace/remove a different plugin in a future config, which could include new plugins (and thus change the index). An alternative would be to replace the entire concerned plugin section, but then again we would not automatically get the new plugins (like with the externalized config). Still wondering if there's not another alternative.

@landryb
Copy link
Member

landryb commented May 30, 2022

Agreed, but on the other hand, this would allow us to disconnect mapstore versions from geOrchestra versions... which is highly desirable IMO.

i dont think that's a good excuse, because right now there's zero adherence between mapstore versions and georchestra versions. Anyone is free to use whatever version of mapstore with whatever version of georchestra, the only risk is using the wrong localConfig.json with the wrong default list of plugins.

And if from what is proposed (using json patches) one cant properly manage plugin configs (which is from my understanding the main motivation for customizing localConfig.json, that and default catalog configs) because it's too fragile, then it's not helping sadly..

heck, it would have taken less of my time to fix #238 by backporting the 3 small commits here and there instead of replying to this issue ;)

Or.. team up with geosolutions to write a web interface to manage localConfig.json (and proposes hints/various possible options, a bit like what GN4 admin section does in some areas), that would generates the patches ? ;)

@catmorales
Copy link
Contributor

I don't have time now to study more this proposal and its impacts.
But is this compatible with adding externel plugins like "signalling" in the localconfig.json as we do actually ?

I am afraid (after reading quickly) that it complicates the configuration which is already not simple ...
Currently, we have the possibility to do what we want and it is important for us not to systematically activate all the plugins or to add external ones.

@Gaetanbrl
Copy link
Contributor

remove localConfig.json and pluginsConfig.json from datadir

Maybe i have a misunderstanding, but the only one good reason to use a merge system (with patch), is to simplify the localConfig.json migration / evolution to use patch and don't change basic localConfig.

Else, it's a nightmare to maintain / understand (after 2 years it's a fog around so many "basics" questions... ). So, i'm not sur it's realy more easy to have others patch files / others externalize stuff...

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

No branches or pull requests

5 participants