Create config per zone

[pierresegonne]

Description

This PR changes the configuration system to support zone configuration as a file per zone.

The configuration is in .yaml for better readability and those yaml files are parsed and merged to generate the same config objects that we are currently using.

I'd suggest doing some subsequent cleaning work to make the configuration objects be more intuitive.

I also suggest removing the current config file in a subsequent PR to minimise the risk of breaking changes.

Preview

Example for a zone for which the configuration is pretty simple: BD.yaml

bounding_box:
- - 87.52178959200009
  - 20.23871491100003
- - 93.14285119700017
  - 27.123544007000064
contributors:
- systemcatch
- alixunderplatz
parsers:
  production: BD.fetch_production
timezone: Asia/Dhaka

And for an exchange, AT->HU.yaml

lonlat:
- 16.605363
- 47.444264
parsers:
  exchange: ENTSOE.fetch_exchange
  exchangeForecast: ENTSOE.fetch_exchange_forecast
rotation: 120

[VIKTORVAV99]

Jeez that is a lot of changed files... Although I think this is a good change overall I'm just curious if this could implement the capacity changes I suggested in #4396 but modified for individual files instead?

Also don't forget to run prettier, thats a lot of warnings... 😅

[VIKTORVAV99]

I noticed a problem with the file names, Windows will not let you put > in the name and honestly I have no idea what will happen if you try and open a file with it. So while I am in favor of this change this needs to be addressed before it's merged from the codebase otherwise I'm afraid it will exclude a lot of people from contributing to the repo.

UPDATE: Git even errors out when trying to check out the branch. 😐

[pierresegonne]

Although I think this is a good change overall I'm just curious if this could implement the capacity changes I suggested in https://github.com/electricitymap/electricitymap-contrib/issues/4396 but modified for individual files instead?

Yeah I can take that into account, we could even split up zones, exchanges, capacities, emission_factors, fallback_zone_mixes

[pierresegonne]

Windows will not let you put > in the name and honestly I have no idea what will happen if you try and open a file with it.

Is it because of the > or because you didn't provide an extension file?

Otherwise I'll find a way around

[VIKTORVAV99]

Windows will not let you put > in the name and honestly I have no idea what will happen if you try and open a file with it.

Is it because of the > or because you didn't provide an extension file?

Otherwise I'll find a way around

It's cause of the > it's a reserved character in windows filenames.

Use any character in the current code page for a name, including Unicode characters and characters in the extended character set (128–255), except for the following:

The following reserved characters:

< (less than)
> (greater than)
: (colon)
" (double quote)
/ (forward slash)
\ (backslash)
| (vertical bar or pipe)
? (question mark)
* (asterisk)

Fun fact: Windows work fine without file extensions when manually open a file with a program (that supports it) like vs code.

[VIKTORVAV99]

Although I think this is a good change overall I'm just curious if this could implement the capacity changes I suggested in https://github.com/electricitymap/electricitymap-contrib/issues/4396 but modified for individual files instead?

Yeah I can take that into account, we could even split up zones, exchanges, capacities, emission_factors, fallback_zone_mixes

It would be nice to have everything related to a zone in its 'zone config' file. Go for it if you have the time. 👍🏻

[pierresegonne]

It would be nice to have everything related to a zone in its 'zone config' file. Go for it if you have the time. 👍🏻

You mean not splitting up the config then? I thought that the other issue was suggesting to split away capacity config into its own config file.

[Kongkille]

I like this a lot for readability and smaller files will also help on version control / merge conflicts.

I'd like to investigate a bit the following

• Any issues reading in the frontend? I haven't tried to import yaml files before.
• Any performance considerations when reading this many files?
• Any performance considerations when working with this many files (eg file search in VSCODE)

I am also wondering if we'd actually need the exchanges in seperate files or if maybe we can just lump all those together? We are not changing those as much so the benefit of separating them out may not be as much.

[VIKTORVAV99]

It would be nice to have everything related to a zone in its 'zone config' file. Go for it if you have the time. 👍🏻

You mean not splitting up the config then? I thought that the other issue was suggesting to split away capacity config into its own config file.

I don't think we need capacities separate if we have one config file for each zone. Like in this PR, one of the motivations to split it out was that if we supported historical capacities the zone.json file would be huge and hard to navigate. But splitting it up so there is one file per zone already solves that.

[VIKTORVAV99]

I like this a lot for readability and smaller files will also help on version control / merge conflicts.

I'd like to investigate a bit the following

• Any issues reading in the frontend? I haven't tried to import yaml files before.
• Any performance considerations when reading this many files?
• Any performance considerations when working with this many files (eg file search in VSCODE)

I am also wondering if we'd actually need the exchanges in seperate files or if maybe we can just lump all those together? We are not changing those as much so the benefit of separating them out may not be as much.

I haven't checked if it's needed yet but there is https://www.npmjs.com/package/yaml-loader to load and bundle yaml.

It should just function as JSON after that.

However I'm not totally sure yaml is better than JSON for this case, it might be better from a pure readability perspective but then you'll have to take whitespace into account when editing. There are pros and cons to both though.

If they are bundled there should be no major preformance issues in the front-end at least.

As for searching with vscode it will definitely have a overhead but I think it might be worth it cause it will be easier to find things manually. At least if you know what you are looking for.

[VIKTORVAV99]

Windows will not let you put > in the name and honestly I have no idea what will happen if you try and open a file with it.

Is it because of the > or because you didn't provide an extension file? Otherwise I'll find a way around

It's cause of the > it's a reserved character in windows filenames.

Use any character in the current code page for a name, including Unicode characters and characters in the extended character set (128–255), except for the following: The following reserved characters:

< (less than)
> (greater than)
: (colon)
" (double quote)
/ (forward slash)
\ (backslash)
| (vertical bar or pipe)
? (question mark)
* (asterisk)

Fun fact: Windows work fine without file extensions when manually open a file with a program (that supports it) like vs code.

I think the easiest would be to change the exchange filenames from zoneKey1->zoneKey2 to zoneKey1_to_zoneKey2, what do you think?

[pierresegonne]

I think the easiest would be to change the exchange filenames from zoneKey1->zoneKey2 to zoneKey1_to_zoneKey2, what do you think?

I'm going for unicode right arrow instead: "→" It's more graphic but could be a bit confusing

[VIKTORVAV99]

I think the easiest would be to change the exchange filenames from zoneKey1->zoneKey2 to zoneKey1_to_zoneKey2, what do you think?

I'm going for unicode right arrow instead: "→" It's more graphic but could be a bit confusing

Should be fine, otherwise I have a feeling we'll find out... 😅

[pierresegonne]

Ok should be good now FYI with this current version, we're locking the new config per zone to the old zones.json (and same for exchanges and co2eq parameters) I'm doing this to minimise the risk of a breaking change, but that means that I'll have to follow-up (next week) with a follow up which consists in cleaning all old references to zones.json that were not addressed here, and keeping the new config files as the single truth

[VIKTORVAV99]

I think we need a postinstall script to the files are generated when the dependencies are installed for the CI to work properly (or we need to change the CI but the first option would be better).

[pierresegonne]

So now, we reduce the need for contributors to worry about the aggregation of the configs:

• It's run officially at build time
• For any other command that require it (develop, build, test etc) the command is included in the script] The files are ignored by the version control, so we only keep the version built at build time as the official one

The potential weakness I see is if people try to circumvent the yarn commands, and end up with a confusing error message. Might thus be worth adding a piece of documentation that explains the whole setup, but I'm not sure yet where is the most appropriate place to do that

[VIKTORVAV99]

@pierresegonne I don't think the script need to be added to that many places, I think it would be enough to add a prebuild, predevelop and postinstall script. But this works for now. 🙂

[pierresegonne]

I think it would be enough to add a prebuild, predevelop and postinstall script

Wasn't aware that such a thing existed 😅

[VIKTORVAV99]

Postinstall is working great!

[VIKTORVAV99]

I think it would be enough to add a prebuild, predevelop and postinstall script

Wasn't aware that such a thing existed 😅

It's super handy, just take any existing script and add a new script with pre or post and they will chain. You can add both if you just need something temporary.

I think we can simplify a lot of our scripts this way but that's something for another PR. 🙂

[VIKTORVAV99]

@pierresegonne I think this line in the dockerfile is causing the CI to fail:

    volumes:
      - './config:/home/config'

It does not match the path for the web directories at:

    volumes:
      - './config:/home/config'
      - './web/.eslintrc:/home/src/electricitymap/contrib/web/.eslintrc'
      - './web/geo:/home/src/electricitymap/contrib/web/geo'
      - './web/public/locales:/home/src/electricitymap/contrib/web/public/locales'
      - './web/locales-config.json:/home/src/electricitymap/contrib/web/locales-config.json'
      - './web/package.json:/home/src/electricitymap/contrib/web/package.json'
      - './web/server.js:/home/src/electricitymap/contrib/web/server.js'
      - './web/src:/home/src/electricitymap/contrib/web/src'
      - './web/views:/home/src/electricitymap/contrib/web/views'
      - './web/webpack.config.js:/home/src/electricitymap/contrib/web/webpack.config.js'

EDIT: Just saw you fixed it already, never mind me. 😅

[VIKTORVAV99]

@pierresegonne did you ever get around to changing the -> in the file name to the unicode arrow?

[pierresegonne]

@pierresegonne did you ever get around to changing the -> in the file name to the unicode arrow?

Yes it's the case now :)

[VIKTORVAV99]

@pierresegonne did you ever get around to changing the -> in the file name to the unicode arrow?

Yes it's the case now :)

Great! 🎉 Thanks for making the change. 🙂

[pierresegonne]

@FelixDQ or @Kongkille can I get your review please?

FYI @tonypls @madsnedergaard @mathilde-daugy

[madsnedergaard]

This PR breaks Chrome when trying to review it 😄 I'd like us to map out the different user journeys for working/changing these configurations to better understand the impact of the proposed changes here - unless you've already done some thoughts on this we could sit down with a whiteboard together? :)

[VIKTORVAV99]

This PR breaks Chrome when trying to review it 😄 I'd like us to map out the different user journeys for working/changing these configurations to better understand the impact of the proposed changes here - unless you've already done some thoughts on this we could sit down with a whiteboard together? :)

I'd recommend using GitHub Pull Requests and Issues and reviewing it locally in VS Code.

783 files changed are a large diff but it seems to be able to handle it without issues!

[pierresegonne]

I'd like us to map out the different user journeys for working/changing these configurations to better understand the impact of the proposed changes here - unless you've already done some thoughts on this we could sit down with a whiteboard together? :)

Sure let's discuss today :)

[pierresegonne]

@VIKTORVAV99 have you tested with the unicode arrows?

[VIKTORVAV99]

@VIKTORVAV99 have you tested with the unicode arrows?

Not much more than I can actually check out the pr now without errors, I've been pretty busy the last few days but I should have time to test it more in depth during the weekend.