Adds a graphql validation script to force deploys #3061

orta · 2018-11-06T23:29:23Z

This script detects breaking changes between Reaction's schema and the one currently on prod.

The current one (where metaphysics prod is a week out of date) looks like:

 ~/d/p/a/j/s/force  $ node scripts/validate_schemas.js

[ { type: 'TYPE_REMOVED', description: 'LatLng was removed.' },
  { type: 'TYPE_REMOVED', description: 'City was removed.' },
  { type: 'TYPE_REMOVED',
    description: 'OrderModeEnum was removed.' },
  { type: 'TYPE_REMOVED', description: 'Offer was removed.' },
  { type: 'TYPE_REMOVED',
    description: 'OfferConnection was removed.' },
  { type: 'TYPE_REMOVED', description: 'OfferEdge was removed.' },
  { type: 'TYPE_REMOVED',
    description: 'ArtworkVersion was removed.' },
  { type: 'TYPE_REMOVED',
    description: 'CreateOfferOrderWithArtworkInput was removed.' },
  { type: 'TYPE_REMOVED',
    description: 'CreateOfferOrderWithArtworkPayload was removed.' },
  { type: 'TYPE_REMOVED',
    description: 'InitialOfferInput was removed.' },
  { type: 'TYPE_REMOVED',
    description: 'InitialOfferPayload was removed.' },
...

orta · 2018-11-06T23:35:13Z

Alternative take, should this happen on a PR to release instead?

orta · 2018-11-06T23:50:28Z

I switched this to use Danger to give a warning instead

orta · 2018-11-07T02:22:16Z

Looks like Danger got removed from CI, so I've added back travis for running Danger on this repo

ashkan18 · 2018-11-07T19:33:50Z

This is really cool! thanks @orta as always!

ashkan18 · 2018-11-09T16:18:49Z

this is really useful!! merge on green

…k_gql_breaking

dleve123 · 2018-11-19T16:58:09Z

.circleci/config.yml

@@ -9,7 +9,7 @@ jobs:
      - checkout
      - run:
          name: Acceptance Tests
-          command: 'yarn install && yarn acceptance src/test/acceptance/*.js'
+          command: "yarn install && yarn acceptance src/test/acceptance/*.js"


random q - is there any reason for this diff? Do we prefer single quoted strings in circle ci config files?

prettier does yml files, so these are the same code standard as the rest of front-end apps

dleve123 · 2018-11-19T17:01:15Z

.travis.yml

@@ -0,0 +1,2 @@
+language: node_js
+script: yarn danger ci


@orta I see that in #3061 (comment) you mentioned that danger got removed from CI, but can you elaborate? I'm just wondering if there's a way that we can modify the configuration for Circle than introduce another build system for this project.

Yep, could be done, but it's simpler this way as it doesn't have to run inside docker and Travis is more reliable than Circle for Danger (due to the way they both handle ENV vars)

https://github.com/danger/danger-js/blob/f648c8a9d9e0bf1710bc6de9b78ef6fe54829066/source/ci_source/providers/Circle.ts#L4-L23

That said, this could also be ported to peril (instead of looking in the node modules, pull out the GraphQL schema from the git repo) but this should be enough for now.

Hah, nice, looks like that is the way to go - the graphql schema for reaction was removed last week, artsy/reaction#1631

dleve123 · 2018-11-19T17:06:27Z

dangerfile.ts

+  }
+
+  // Breaking change check for Metaphysics production when deploying
+  if (danger.github.pr.base.ref === "release") {


I know a conversation topic during our recent platform practice meeting was checking for "dependencies in prod" before code hits staging as well, as such, ensuring that we never have staging code that shouldn't be deployed production.

Thoughts on updating this to run on PRs to master -- providing the staging check?

I'm interested in this^ too, but am OK with enforcing this on staging->release merges to start (and we can extend based on how that goes and how often it's an obstacle).

I'm wary that this will "break" builds that have nothing to do with metaphysics, so happy to do this as-is now and can look at making a tighter loop for when people either update reaction or other builds

dleve123 · 2018-11-19T17:11:28Z

scripts/validate_schemas.js

+  buildClientSchema,
+  printSchema,
+  buildSchema,
+  findBreakingChanges,


so there's first class support for this in graphql-js? Very cool!

Yeah, I was surprised too!

orta · 2018-12-13T22:34:49Z

What it looks like right now, if you remove the deploy PR check:

WIP on adding a graphql validation script to force deploys

c84a42e

Move the warning to danger

34b25a4

Test the danger integration with prod

f75e9a1

orta force-pushed the wip_check_gql_breaking branch 9 times, most recently from b077074 to 5e0341b Compare November 7, 2018 13:04

orta mentioned this pull request Nov 7, 2018

Make sure that logs are shwon from inside the runner danger/danger-js#758

Merged

orta force-pushed the wip_check_gql_breaking branch 6 times, most recently from 85c451b to 1aa3e73 Compare November 7, 2018 18:13

orta force-pushed the wip_check_gql_breaking branch from 1aa3e73 to b6de4a9 Compare November 7, 2018 19:51

orta assigned ashkan18 Nov 9, 2018

orta changed the title ~~WIP on adding a graphql validation script to force deploys~~ Adds a graphql validation script to force deploys Nov 9, 2018

Adds travis for Danger

f2cab46

orta force-pushed the wip_check_gql_breaking branch from b6de4a9 to f2cab46 Compare November 9, 2018 15:56

Merge branch 'master' of https://github.com/artsy/force into wip_chec…

d8e9c96

…k_gql_breaking

orta mentioned this pull request Nov 15, 2018

RFC: Ensure dependent services are set up in production before allowing a PR requiring schema changes to be merged artsy/README#109

Closed

dleve123 reviewed Nov 19, 2018

View reviewed changes

Merge master

5a862ec

orta force-pushed the wip_check_gql_breaking branch from 8014c71 to 999d53a Compare December 13, 2018 21:50

Update wording

a901042

orta force-pushed the wip_check_gql_breaking branch from 999d53a to a901042 Compare December 13, 2018 21:52

Show what it would be like if it were deployed today

5bdfaa6

Bring back the release check

e59b7b3

orta merged commit 2cc25bc into master Dec 13, 2018

bhoggard deleted the wip_check_gql_breaking branch April 8, 2020 20:45

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Adds a graphql validation script to force deploys #3061

Adds a graphql validation script to force deploys #3061

orta commented Nov 6, 2018 •

edited

Loading

orta commented Nov 6, 2018 •

edited

Loading

orta commented Nov 6, 2018

orta commented Nov 7, 2018

ashkan18 commented Nov 7, 2018

ashkan18 commented Nov 9, 2018

dleve123 Nov 19, 2018

orta Nov 19, 2018

dleve123 Nov 19, 2018

orta Dec 13, 2018

orta Dec 13, 2018

dleve123 Nov 19, 2018

joeyAghion Dec 13, 2018

orta Dec 13, 2018

dleve123 Nov 19, 2018

orta Dec 13, 2018

orta commented Dec 13, 2018

Adds a graphql validation script to force deploys #3061

Adds a graphql validation script to force deploys #3061

Conversation

orta commented Nov 6, 2018 • edited Loading

orta commented Nov 6, 2018 • edited Loading

orta commented Nov 6, 2018

orta commented Nov 7, 2018

ashkan18 commented Nov 7, 2018

ashkan18 commented Nov 9, 2018

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

orta commented Dec 13, 2018

orta commented Nov 6, 2018 •

edited

Loading

orta commented Nov 6, 2018 •

edited

Loading