-
Notifications
You must be signed in to change notification settings - Fork 0
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
Harvest and transform FOLIO json schemas into a base GraphQL schema #71
Conversation
c896bd5
to
0707674
Compare
ca8a9d4
to
e7b6029
Compare
I ran the following commands on this branch on got the error: ➜ folio-graphql git:(consume-folio-json-schemas) npm install
added 55 packages, changed 1 package, and audited 906 packages in 2s
➜ folio-graphql git:(consume-folio-json-schemas) npm run update-graphql-schema
> sul-folio-graphql-server@1.0.0 update-graphql-schema
> node scripts/generate-graphql-from-json-schemas.js json-schemas/
/Users/marloelilongley/Projects/folio-graphql/node_modules/@lifeomic/json-schema-to-graphql-types/src/converter.js:116
const modifiedType = includes(schema.required, attributeName) ? GraphQLNonNull(type) : type;
^
TypeError: Class constructor GraphQLNonNull cannot be invoked without 'new'
at /Users/marloelilongley/Projects/folio-graphql/node_modules/@lifeomic/json-schema-to-graphql-types/src/converter.js:116:71
at /Users/marloelilongley/Projects/folio-graphql/node_modules/lodash/mapValues.js:38:34
at /Users/marloelilongley/Projects/folio-graphql/node_modules/lodash/_createBaseFor.js:17:11
at baseForOwn (/Users/marloelilongley/Projects/folio-graphql/node_modules/lodash/_baseForOwn.js:13:20)
at mapValues (/Users/marloelilongley/Projects/folio-graphql/node_modules/lodash/mapValues.js:37:3)
at getObjectFields (/Users/marloelilongley/Projects/folio-graphql/node_modules/@lifeomic/json-schema-to-graphql-types/src/converter.js:112:5)
at fields (/Users/marloelilongley/Projects/folio-graphql/node_modules/@lifeomic/json-schema-to-graphql-types/src/converter.js:146:23)
at resolveObjMapThunk (/Users/marloelilongley/Projects/folio-graphql/node_modules/graphql/type/definition.js:504:40)
at defineFieldMap (/Users/marloelilongley/Projects/folio-graphql/node_modules/graphql/type/definition.js:766:20)
at GraphQLObjectType._fields (/Users/marloelilongley/Projects/folio-graphql/node_modules/graphql/type/definition.js:691:26)
Node.js v18.8.0 |
8398a00
to
4aa2e95
Compare
Oops - I forgot I made some local changes to the upstream library (which isn't compatible with the latest graphql... they have a ticket upstream..) you've got to go in and add a couple |
OK will do -- but just so I'm clear, are we blocked on merging this until the upstream ticket is handled? |
I hope not. It's only a problem for running the sync script, but we don't need to do that regularly and we can always change a couple files in |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
i imagine you already considered it, but would using git submodules alleviate the problem of tracking the ref for each of the folio repos we want to pull in? there might be a clever way to specify that we only want shallow clones of each of them, and only some files, so that we don't need to commit all the JSON itself to this repo
@thatbudakguy : if you feel strongly, maybe we can ticket that for investigation? I didn't bother because FOLIO is already a mess of submodules and the repositories themselves are pretty big. For as often as we'll need to sync, I'm not sure it's worth making things even more complex just yet. |
i don't feel strongly, just wanted to check if you'd already investigated — i'm cool with this approach |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Works!
This PR uses the upstream JSON schemas to generate (the start of) a base GraphQL schema. Previously, our GraphQL schema was hand-crafted, but by automating some of the work we can get a more complete + consistent representation of the upstream APIs and set ourselves up better for consuming upstream updates.
The downside to all this is that the upstream schema expressions are not well-suited for using existing JSON schema to GraphQL schema tooling, including:
$id
properties, leaving the schemas unconnected and poorly named$ref
properties (or FOLIO-custom expressions) that expect to operate on a local file system (?)Account
would be the base model frommod_patron
, andAccountdata
would be the model for/accounts
)After all that, we still need to extend the generated schema with linking properties to connect various models together.
Perhaps something like #64 would let us keep the upstream-generated code fully separate from our enhancements.
As of this writing, the upstream schema to graphql converter isn't compatible with graphql 16, but it just needs a couple changes
With those changes, you can run the
npm run update-graphql-schema
script and get the graphql output you can merge into theschema.graphql
file