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

Move to using YAML for project layers file #711

Closed
gravitystorm opened this issue Jul 10, 2014 · 6 comments · Fixed by #947
Closed

Move to using YAML for project layers file #711

gravitystorm opened this issue Jul 10, 2014 · 6 comments · Fixed by #947

Comments

@gravitystorm
Copy link
Owner

The situation with our SQL queries has become a nightmare for handling pull requests. In the .mml file, the entire query needs to be on one line, and some of these are becoming thousands of characters long (the current longest clocks in at 2533 characters - or over 30 lines if we were doing 80 chars per line). Recent work suggests the queries could become even longer.

Due to their length, any pull request that changes the SQL - even just to add a since value to an existing list - is both hard to review, and also pull requests will frequently become unmergeable since any change to the query will cause a merge conflict.

Of course, this limitation of using json files is well known, which is why tm2 uses yaml files instead. An example of a multiline query can be seen here. I can say from personal experience it's much more pleasant to edit, and given the number of contributors + pull requests for this project, it's becoming urgent.

However, I don't want to change this project to become incompatible with tilemill or to require a custom fork or whatnot. So as far as I see the options are:

  • Create a plugin for tilemill to support project.yml files
  • Add project.yml support to tilemill directly, and wait for a subsequent release

I don't know enough about how the tilemill plugins work, but that will be the first thing to investigate.

@matthijsmelissen
Copy link
Collaborator

See also my (naive) request at mapbox/carto#336.

@pnorman
Copy link
Collaborator

pnorman commented Aug 27, 2014

So, mapbox-studio has been released, which has saner MML files, but I see three big issues

  1. I don't like the idea of requiring software that requires a mapbox account and calls home with analytics (--rogue mapbox/mapbox-studio-classic#592) to edit the stylesheet
  2. mapbox-studio is focused around vector tiles, which are not a technology where the server-side tools are ready (renderd, mod_tile, etc) and which offer few to no advantages in a single-style situation. It's not obvious to me if we have to be using vector tiles with the resulting stylesheet or not, as I've lately been professionally focused around vector tile rendering
  3. I'm not sure we can process a TM2 source or source + style with carto to get a resulting XML stylesheet. This is kind of important so that we can generate the XML to render server-side.
  4. You can't edit TM2 sources through the UI anyways as it will clobber comments (Saving source clobbers comments in YAML mapbox/mapbox-studio-classic#655) and anything else it doesn't understand as TM2 converts the YAML to JSON internally, then on save converts the JSON to YAML to serialize. This is probably unsolvable, as it's similar to the normal problem with a config file that can be edited with a UI or by hand.

@pnorman
Copy link
Collaborator

pnorman commented Aug 28, 2014

In a way, switching to hstore also depends on this. The switch will involve significant query rewriting, and I've been put off from that by the thought of editing that many queries in json.

@matthijsmelissen
Copy link
Collaborator

Would switching to hstore not be backwards compatible if we keep the same columns in the style file?

@pnorman
Copy link
Collaborator

pnorman commented Sep 8, 2014

Would switching to hstore not be backwards compatible if we keep the same columns in the style file?

I thought I wrote something on this, but I guess not.

Simply adding hstore is backwards compatible, but is a performance loss. We can regain that by changing queries and dropping columns. It is possible to do this incrementally - but that's a separate issue.

@pnorman
Copy link
Collaborator

pnorman commented Sep 8, 2014

I did some investigation

python -c 'import sys, yaml, json; json.dump(yaml.load(sys.stdin), sys.stdout, indent=4)' will convert YAML to JSON, and I tried round-tripping with cat project.mml | python -c 'import sys, yaml, json; yaml.dump(json.load(sys.stdin), sys.stdout)' | python -c 'import sys, yaml, json; json.dump(yaml.load(sys.stdin), sys.stdout, indent=4)', and the result compiled with carto to XML. There were differences in the XML output, but I think they were ordering differences, which are not important.

Of course, converting is likely to be the diff from hell.

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

Successfully merging a pull request may close this issue.

3 participants