Skip to content
This repository has been archived by the owner on Jun 21, 2022. It is now read-only.

Migrate API groups & interfaces to mdn/data json files #78

Closed
wants to merge 1 commit into from

Conversation

Elchi3
Copy link
Member

@Elchi3 Elchi3 commented Jan 3, 2017

API data (interfaces and groups) change more often and are data, not logic, so they live in the mdn/data repository. This PR changes the macros that use Template:InterfaceData and Template:GroupData to use the JSON files in the mdn/data repo.

A follow-up that syncs the two data macros with the JSON files over at mdn/data should be created, so that we can then remove the two files here afterwards.

Add w=1 to the review URL to ignore whitespace.

@jgmize
Copy link
Contributor

jgmize commented Jan 4, 2017

I recommend against fetching live data from github during rendering on the server side. In addition to the performance impact under even best-case circumstances, this can result in incomplete or invalid content due to network timeouts, rate limiting, etc. These problems would be compounded by caching and DB persistence of rendered pages, amplifying a single request failure on the server side into an incorrectly rendered page for many pageviews.

@Elchi3
Copy link
Member Author

Elchi3 commented Jan 4, 2017

Thanks Josh, this is exactly the kind of "don't do this in KumaScript this way" feedback I hope to see more of, so that we can think about better solutions. :-)
KumaScript calls various end points (not necessarily APIs) like GitHub or Bugzilla this way without major problems in the last months/years, nonetheless this is not a good approach for the reasons you mentioned.

So, why do we want to use external JSON files in a GitHub repo here? We want to separate the data from the scripts, we want to allow external people to also consume our data, and we want to collaborate on this data in an own repo. This has worked well over at mdn/data.

Now, how do we get back that data into KumaScript? So far, we saw two options:

  1. Duplicate the files in the data repo and the (wiki) macros, or
  2. Load the files directly from GitHub.

As 2. showed no problems so far, this was the compelling way to go about this.

How would you load external JSONs into KumaScript scripts?
Is a better approach to this blocking this PR and should we remove other scripts that load from GitHub?

@jgmize
Copy link
Contributor

jgmize commented Jan 4, 2017

With the current deployment system, duplicating the files and keeping them updated manually is probably the best option for now IMO, but once MDN (including kumascript) is being continuously deployed to Kubernetes later this year we'll have much better options for automatically updating data from external repos and APIs on the server side, either as part of an automated deployment, for example as part of the kumascript Docker image build phase, or as a sidecar container like https://github.com/kubernetes/git-sync, ideally with some sort of schema validation.

You may not have noticed problems loading data from external sources like github during the render phase yet, but that doesn't mean it hasn't already happened and you just haven't noticed it, or that it won't become a more serious issue in the future.

@Elchi3 Elchi3 removed the request for review from SebastianZ January 5, 2017 10:23
@Elchi3
Copy link
Member Author

Elchi3 commented Jan 5, 2017

Thank you Josh, I filed https://bugzilla.mozilla.org/show_bug.cgi?id=1328839 to find a solution.

This is WONTFIX then.

@Elchi3 Elchi3 closed this Jan 5, 2017
@Elchi3 Elchi3 deleted the migrate-groups-ifs branch January 5, 2017 10:39
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants