feat: schema differ by Birkbjo · Pull Request #43 · dhis2/cli

Birkbjo · 2019-05-10T12:24:13Z

Adds 'schema' namespace with command 'diff' under utils. I don't know if we want this in its own repo, or directly in utils like I did here. Would not be hard either way to extrat to its own repo or package.

The background for making this is that we often get weird bugs (especially in the maintenance app) when seemingly small changes to properties in the schemas (like setting ie embedded: false) are made. They are hard and time consuming to debug, and we frontend developers always get the blame for broken apps, even if the cause is an undocumented API change.

The idea is that we can run this tool to grab and generate a diff of the schemas. It outputs an html file, with all the schemas and delta-data embedded (much like source-map-explorer), so its easily sharable as a static resource.

I'm not sure if this is going to be run in the build pipeline. I initially created a prototype to debug a weird bug (https://jira.dhis2.org/browse/DHIS2-6767) , and then decided to port it here, as I believe its a valuable utility.

I tried to minimize typing long urls, so I made the play-server the default base-url, and that will be used if the --base-url does not have an argument.
So you can do this: d2 utils schema diff /2.31dev /2.32 -b.

Example diff: https://birk.dev/static/diffs/2.32.0_a9f597b__2.33-SNAPSHOT_9d8bb91.html

amcgee · 2019-05-10T15:34:52Z

This looks awesome @Birkbjo, really useful!! I'll take a closer look before Monday :-)

varl · 2019-05-14T06:59:15Z

I expect that the default output of the command would be rendered straight to the terminal in a diff format similar to the diff command; unless that tool can be used, in which case that is preferable to me.

I don't like when tools write to disk automatically either, so I'd prefer to redirect > to a file if I need the output saved.

So my preferred usage (based on what I expect from a terminal application) would be:

d2 utils schema diff /2.31 /2.32
+ fieldName
- fieldName

d2 utils schema diff /2.31 /2.32 > result.diff

# render using `diff`
d2 utils schema diff /2.31 /2.32 | diff

d2 utils schema diff /2.31 /2.32 --json
{
    ...
}

d2 utils schema diff /2.31 /2.32 --json > result.json

d2 utils schema diff /2.31 /2.32 --html
<html>
...
</html>

d2 utils schema diff /2.31 /2.32 --html > result.html

# to open in default browser (linux)
d2 utils schema diff /2.31 /2.32 --html > result.html | xdg-open

varl

Left a comment with my CLI tool preferences.

amcgee · 2019-05-14T09:04:50Z

I think I agree @varl about having CLI-first options, though I don't mind opening a browser with a --open flag, for instance. I think I prefer --format html to --html but not super strongly.

varl · 2019-05-14T11:34:22Z

--format could be nice just to minimize the amount of switches.

I'm OK with --open existing as a switch, I can still pipe it to different browsers if I need to through the saved .html file.

amcgee · 2019-05-14T15:19:45Z

https://github.com/sindresorhus/open might be useful

Birkbjo · 2019-05-14T15:26:20Z

Did a refactor now. --format instead of --html and --json. Now you can
d2 utils schema diff /2.31 /2.32 -b --format html > result.html

By default it outputs to console, however I also added --output and -o to be able to generate the 'identifier'-name automatically. This outputs the filename, so you can do d2 utils schema diff /2.31 /2.32 -b --format html -o | xargs open

Some thoughts about the rendering:
Previously all the data is outputted to html, and the formatting happens client side (by injecting the jsondiffpatch dep into the html).

Pros:

You can inspect the data in the console
You have access to jsondiffpatch in the console, so you can format and patch as you wish

Cons:

Bigger file size (we inject data and jsondiffpatch in the HTML)
Minor loading time while the browser formats

I refactored this to generate and output just the diff to html. This means a reduction of ~90% of the outputted html size. From ~5MB to ~400kb. This means we now lose the original data, so you cannot see the data that was not changed, but I believe the reduction in size is well worth it.

Birkbjo · 2019-05-14T15:29:06Z

https://github.com/sindresorhus/open might be useful

I've looked into it. But I don't really want to add more dependencies than necessary, so I believe piping to xargs open or what other program your distro has is sufficient.

varl

Some nits to pick other than that I think this looks good.

amcgee

Hey @Birkbjo - thanks for putting this together, I love the CLI option descriptions in help with sane defaults and intelligent prompt fallbacks!

I'm having some trouble getting a diff, though. Here's what I get when I try a few commands which I would expect to work:

No default to play which I think is fine but contradicts the description of this PR

Passing a baseurl parameter doesn't seem to work and throws a nasty rejected promise error. At minimum can we fix this so that it shows a more user-friendly message? Also, though, I would expect this command to work unless I'm misunderstanding something.

Finally, passing absolute URLs doesn't seem to work either. When I got to https://play.dhis2.org/2.32dev/api/schemas.json I se a huge JSON with a schemas key but no meta key.

Am I missing something here?

amcgee · 2019-05-15T09:28:28Z

+                type: 'input',
+                name: 'username',
+                message: `Username for ${url}`,
+                default: 'admin',


This is fine for now but at some point we'll probably want to "centralize" the defaults for cli tools (where to find the server you want to talk to, what auth to use), so we can easily change/remove them or at least make them configurable.

Birkbjo · 2019-05-15T17:19:51Z

Added support for fetching schemas, and saving these to disk. These files can be used as an input for the differ.
Sorry for all the refactoring... Had some common code I wanted to share. I was abit unsure of the folder structure, so don't hesitate to state your opinions about that. Maybe the the common schema code be moved to 'support' folder as well?

* chore(package): update dependencies * chore(package): update dependencies * chore(package): update dependencies * chore(package): update dependencies * chore(package): update lockfile yarn.lock * chore: regenerate yarn.lock

amcgee

Few minor comments, looking good will test soon!

amcgee · 2019-05-17T11:52:19Z

+            })
+            .option('auth', {
+                type: 'string',
+                describe: `Auth to use, formatted as "user:password". Note that this is not safe, as password is shown in history.


Should we just disable passing user/password on the command line and always prompt if env vars aren't set?

I think this would be a nice simplification.

varl · 2019-05-21T14:38:51Z

I think that default should be to play, and the usage of the --base-url/-b would be to specify the base URL, e.g.:

d2 utils schema diff /2.31 /2.32 hits play.dhis2.org.

d2 utils schema diff /2.31 /2.32 -b dhis2.vardevs.se hits my server.

d2 utils schema diff dhis2.vardevs.se/2.31 play.dhis2.org/2.32 diffs the two different servers.

varl · 2019-05-28T06:26:47Z

Use cases for this keeps popping up.. 👍

amcgee · 2019-05-28T09:54:39Z

This is looking really great @Birkbjo !! I tested and it's working well, really love the HTML formatting of the diff. The following are the final minor things, then we should be able to get this merged

One question: why are there all these "array reorder" diffs in what should be "identical" (or nearly identical) schemas? Is array ordering non-deterministic in the backend or are these legit scheme divergences.

And a couple future considerations for the CLI interface, probably not blocking this merge:

--base-url and --auth both apply to BOTH servers, which might be annoying when diffing i.e. play with a local server. This can be worked around effectively by downloading schemas independently with d2 utils schema fetch and then diffing the downloaded files, so I don't think this is urgent.
I did find it somewhat cumbersome to "discover" the mechanism for generating and viewing the HTML diff, so it might be worth some documentation somewhere?

All in all really awesome tool, I think it will be incredibly useful. Great work!

amcgee

Hey @Birkbjo ! Is this ready to go or are you still making some changes?

amcgee · 2019-06-03T10:41:30Z

+    }
+    const schemaFileName = `${schemaIdentifier(meta)}.json`
+    const loc = await cache.get(schemasUrl, schemaFileName, {
+        raw: true,


Hey @Birkbjo I think this raw: true is unnecessary here, since shemasUrl isn't an archive .tar or .tar.gz file - see cache source. It shouldn't hurt anything to leave it in if you want to be explicit, but wanted to point out that its technically superfluous

Thanks for letting me know!

I still have a few finishing touches to do. Have been busy with some Maintenance-issues lately.

Ok no worries, just ping me when it's ready!

@amcgee , I think this is coming together now! Would appreciate a new peek, especially the docs, as things should hopefully be a bit clearer now.
Note that I removed the play-server from defaults, as I've added config-support.

amcgee · 2019-08-05T08:57:14Z

@Birkbjo cool, I'll take a look at this in the next couple days!

amcgee

Hey @Birkbjo! I found a few bugs which prevent this tool from being used if you haven't set up a config file, commented on a couple specific ones. I can re-test once those are fixed!

amcgee · 2019-08-05T09:20:59Z

+}
+
+function resolveConfig(args) {
+    if (!args.utils && !args.utils.schema) {


This should be ||, not && - I get this error

TypeError: Cannot read property 'schema' of undefined at resolveConfig (/Users/awm/dev/dhis2/cli/d2/packages/utils/src/cmds/schema/index.js:47:36)

amcgee · 2019-08-05T09:37:58Z

+}
+
+async function getAuthHeader(url, { auth }) {
+    let { username, password } = auth


This breaks if the tool is used without a config file, which we should support. After changing && to || above I get this error:

./packages/utils/bin/d2-utils schema --auth diff https://play.dhis2.org/dev http://localhost:8080 d2-utils schema diff <leftUrl> <rightUrl> Generate a diff of schemas between DHIS2 versions Positionals: leftUrl the url to the running DHIS2 server, should have schemas available relative to this at /api/schemas.json Can also be a JSON-file with schemas. See schema fetch [string] [required] rightUrl the url to another running DHIS2 server, should have schemas available relative to this at /api/schemas.json Can also be a JSON-file with schemas. See "utils schema fetch" [string] [required] Options: --version Show version number [boolean] --config Path to JSON config file --force By default each schema is cached, identified by the version and revision. Use this flag to disable caching and download the schemas. [boolean] --auth Prompt credentials for each server. [boolean] [default: false] --base-url, -b BaseUrl to use for downloading schemas. If this is set leftServer and rightServer should be relative to this url, eg. /dev. [string] --output, -o Specify the location of the output. If used as a flag a file relative to current working directory is generated with the name "LEFT-version_revision__RIGHT-version_revision.html". If the location is a directory, the default filename is used and output to location. [string] --format Specify the format of the output [string] [choices: "html", "json", "console"] [default: "console"] --ignore-array-order Sort all nested arrays in schemas. The order of nested arrays are non-deterministic, which may clutter the diff. Enabling this will prevent most internal array moves, which are probably irrelevant anyway. [boolean] [default: false] -h, --help Show help [boolean] TypeError: Cannot destructure property `username` of 'undefined' or 'null'. at getAuthHeader (/Users/awm/dev/dhis2/cli/d2/packages/utils/src/cmds/schema/index.js:70:34) at schemasFromUrl (/Users/awm/dev/dhis2/cli/d2/packages/utils/src/cmds/schema/index.js:95:47) at getSchemas (/Users/awm/dev/dhis2/cli/d2/packages/utils/src/cmds/schema/diff/index.js:47:25) at Object.run [as handler] (/Users/awm/dev/dhis2/cli/d2/packages/utils/src/cmds/schema/diff/index.js:188:31) at Object.runCommand (/Users/awm/dev/dhis2/cli/d2/node_modules/yargs/lib/command.js:242:26) at Object.parseArgs [as _parseArgs] (/Users/awm/dev/dhis2/cli/d2/node_modules/yargs/yargs.js:1078:30) at Object.runCommand (/Users/awm/dev/dhis2/cli/d2/node_modules/yargs/lib/command.js:200:96) at Object.parseArgs [as _parseArgs] (/Users/awm/dev/dhis2/cli/d2/node_modules/yargs/yargs.js:1078:30) at Object.parse (/Users/awm/dev/dhis2/cli/d2/node_modules/yargs/yargs.js:542:19) at module.exports (/Users/awm/dev/dhis2/cli/d2/node_modules/@dhis2/cli-helpers-engine/lib/makeEntryPoint.js:43:11)

This comes originally from diff/index.js:190. I think some more testing of un-configured schema diffing might be in order!

Sorry, I forgot to fallback to prompt if no auth was given. Should be fixed now.

amcgee

Tested this and it's awesome - sorry it took so long to review @Birkbjo, I'll merge as soon as the updated tests pass!

# [2.3.0](v2.2.2...v2.3.0) (2019-09-17) ### Features * schema differ ([#43](#43)) ([c46e343](c46e343)), closes [#51](#51)

dhis2-bot · 2019-09-17T11:42:45Z

🎉 This PR is included in version 2.3.0 🎉

The release is available on:

Your semantic-release bot 📦🚀

Birkbjo added 2 commits May 10, 2019 12:38

feat: dhis-schema differ

8ccd649

fix: remove parse from entrypoint-call

4a25c4b

Birkbjo requested a review from amcgee May 10, 2019 12:24

Birkbjo commented May 10, 2019

View reviewed changes

Comment thread packages/create-app/bin/d2-create-app

Birkbjo commented May 10, 2019

View reviewed changes

Comment thread packages/utils/src/cmds/schema/diff/index.js

fix: missed a .parse()

5128752

Birkbjo force-pushed the feat/schema-diff branch from c877395 to 5128752 Compare May 10, 2019 12:38

fix: array to object, fixing diff bug. refactor to 3 args

d984caf

varl reviewed May 14, 2019

View reviewed changes

Birkbjo added 2 commits May 14, 2019 14:44

Merge branch 'master' into feat/schema-diff

472715d

refactor(schema): format and output switch

bb7480c

perf(schema): generate html, reduce html size by 90%

ead3839

varl reviewed May 14, 2019

View reviewed changes

Comment thread packages/utils/src/cmds/schema/diff/index.js Outdated

varl reviewed May 14, 2019

View reviewed changes

Comment thread packages/utils/src/cmds/schema/diff/utils.js Outdated

varl reviewed May 14, 2019

View reviewed changes

Merge branch 'master' into feat/schema-diff

05ddf48

amcgee suggested changes May 15, 2019

View reviewed changes

amcgee and others added 5 commits May 15, 2019 19:26

chore: add utils package.json to greenkeeper

ac46f5d

chore: update dependencies for default 🌴 (#51)

23542f6

* chore(package): update dependencies * chore(package): update dependencies * chore(package): update dependencies * chore(package): update dependencies * chore(package): update lockfile yarn.lock * chore: regenerate yarn.lock

feat(schema-fetch): add support for fetching schemas. refactoring

b8da4b8

fix: missing require, remove absolute-url check

6e626e0

chore: merge yarn.lock

6efa727

amcgee reviewed May 17, 2019

View reviewed changes

Merge branch 'master' into feat/schema-diff

31118e8

feat: add deep linking. Handle missing rev

8de15f8

Birkbjo force-pushed the feat/schema-diff branch from 3d98f28 to 8de15f8 Compare May 29, 2019 14:34

feat: add option to sort arrays

7166299

amcgee reviewed Jun 11, 2019

View reviewed changes

amcgee and others added 9 commits June 11, 2019 15:09

Merge branch 'master' into feat/schema-diff

326ea96

docs(schema): wip docs and examples for schema diff

22b0f3d

fix: add missing formatter-file

e2e01d4

fix: support for ommiting protocol. Remove -b as flag

38cf623

Merge branch 'master' into feat/schema-diff

4a662d0

feat(schemadiff): support for auth through config

fec113a

feat(schema-fetch): support multiple urls and baseurl

65a966e

fix(schema-fetch): fix baseurl

575d45c

docs(schema): fetch docs

67bf3c5

Birkbjo requested a review from amcgee July 31, 2019 12:16

amcgee suggested changes Aug 5, 2019

View reviewed changes

Birkbjo and others added 3 commits August 9, 2019 16:25

fix(schema): fallback to prompt if no config

2bb2119

Merge branch 'master' into feat/schema-diff

5e2c615

Merge branch 'master' into feat/schema-diff

089bf8a

amcgee approved these changes Sep 17, 2019

View reviewed changes

amcgee merged commit c46e343 into master Sep 17, 2019

amcgee deleted the feat/schema-diff branch September 17, 2019 11:39

dhis2-bot added a commit that referenced this pull request Sep 17, 2019

chore(release): cut 2.3.0 [skip ci]

0f6d304

# [2.3.0](v2.2.2...v2.3.0) (2019-09-17) ### Features * schema differ ([#43](#43)) ([c46e343](c46e343)), closes [#51](#51)

dhis2-bot added the released label Sep 17, 2019

Conversation

Birkbjo commented May 10, 2019 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Uh oh!

Uh oh!

amcgee commented May 10, 2019

Uh oh!

varl commented May 14, 2019

Uh oh!

varl left a comment

Choose a reason for hiding this comment

Uh oh!

amcgee commented May 14, 2019

Uh oh!

varl commented May 14, 2019

Uh oh!

amcgee commented May 14, 2019

Uh oh!

Birkbjo commented May 14, 2019 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Birkbjo commented May 14, 2019

Uh oh!

Uh oh!

Uh oh!

varl left a comment

Choose a reason for hiding this comment

Uh oh!

amcgee left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Birkbjo commented May 15, 2019

Uh oh!

amcgee left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

varl commented May 21, 2019

Uh oh!

varl commented May 28, 2019

Uh oh!

amcgee commented May 28, 2019

Uh oh!

amcgee left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Birkbjo Jul 23, 2019 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

amcgee commented Aug 5, 2019

Uh oh!

amcgee left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

amcgee left a comment

Choose a reason for hiding this comment

Uh oh!

dhis2-bot commented Sep 17, 2019

Uh oh!

Reviewers

Birkbjo commented May 10, 2019 •

edited

Loading

Birkbjo commented May 14, 2019 •

edited

Loading

Birkbjo Jul 23, 2019 •

edited

Loading