Postgraphile config #79
Conversation
ana0
requested review from
i-norden,
elizabethengelman,
Gslaughl,
m0ar,
rmulhol and
yaoandrew
postgraphile/package.json
Outdated
| @@ -7,6 +7,7 @@ | |||
| "lint": "tslint --project ./tsconfig.json --config ./tslint.json", | |||
| "postinstall": "rm -rf node_modules/@graphile && mkdir node_modules/@graphile && cp -R ./vendor/postgraphile-supporter/ ./node_modules/@graphile/plugin-supporter/", | |||
| "start": "npm run build && node ./build/dist/vulcanize-postgraphile-server.js", | |||
| "dev": "./node_modules/typescript/bin/tsc && CONFIG_PATH='./config.toml' node build/dist/src/index.js", | |||
There was a problem hiding this comment.
do we want to set the CONFIG_PATH env variable here? seems like it could override one that's defined by the user, which might be confusing. also it seems like the user may need to create this file since I think it'll be ignored by git?
There was a problem hiding this comment.
It doesn't need to be defined, here, was just for dev purposes - will remove. And yup, currently the user would need to create the config file/the config file is optional
Thanks for the review, will squash 👍 Also, just wanna clarify what you mean about throw an error if Right now it won't throw an error if undefined, with the thinking that it would be good to preserve the ability for the user have no config file and pass in the database variables directly as described in the rest of the readme. Can switch to making |
|
Ah I see, that makes sense - I was thinking that it should throw an error, but I like your idea of allowing the user to pass it in directly without a config file. |
ana0
force-pushed
the
postgraphileConfig
branch
from
0c2cea7 to
12c28e4
Compare
|
Cool :) Was trying to avoid breaking anyone else's set up! |
| database = parsedToml['database']['name']; | ||
| user = parsedToml['database']['user']; | ||
| password = parsedToml['database']['password']; | ||
| schemas = parsedToml['database']['schemas']; |
There was a problem hiding this comment.
do we need to update or modify the code below that's assigning based on process.env?
There was a problem hiding this comment.
Would think we don't need to, since the schemas var defaults to ["public"]. We could at some point, but my understanding is that an array env var will introduce further complications
There was a problem hiding this comment.
Will merge for now, but happy to revisit :)
Thought this would be helpful, as vulcanize opens up to people writing their own transformers