Add local development swiss army knife script scripts/dev.sh
#2547
Conversation
|
Beep boop! 🤖 Hey @jberryman, thanks for your PR! One of my human friends will review this PR and get back to you as soon as possible. 🕐 Stay awesome! 😎 |
|
|
|
Deploy preview for hasura-docs ready! Built with commit a5a0763 |
a1b8018 to
615dee1
Compare
|
Review app for commit 615dee1 deployed to Heroku: https://hge-ci-pull-2547.herokuapp.com |
|
Review app for commit 5279909 deployed to Heroku: https://hge-ci-pull-2547.herokuapp.com |
5279909 to
f510081
Compare
|
Maybe related: #2255 |
|
TODO
|
f510081 to
899a42c
Compare
|
Punting on incorporating pgdump functionality for now. This is ready for review (i.e. please try it out and let me know if it works) . @lexi-lambda if you have time to check it on mac that would be awesome as well. |
scripts/dev.sh scripts/dev.sh
|
Review app for commit 899a42c deployed to Heroku: https://hge-ci-pull-2547.herokuapp.com |
|
Off topic, but: @rakeshkky I'm noticing that those two tests mentioned above seem to pass consistently when I run graphql-engine with |
lexi-lambda
left a comment
There was a problem hiding this comment.
I’ve left a few comments, but this looks generally good to me. It does raise a couple general questions for me, though:
-
We have a Makefile in
server/, which seems to do a handful of things. This script clearly does things the Makefile does not, and vice versa, but it seems a little confusing to have both unless they have more clearly-defined roles. -
This adds a lot of bash code, and I’m generally of the opinion that the less bash in a project the better. I do think this kind of tooling is useful, and I also recognize that it’s not really clear if there’s a technology that can serve this purpose better, so maybe it’s fine. It just gives me pause.
-
From the broader POV, this is an opinionated script, which is good: I like tools that have opinions. The trouble is that I now have a workflow that works for me, and it makes some different choices: I run a non-dockerized postgres instance, for example. That’s not inherently a problem, since obviously nobody is forced to use this script, but I imagine other people might be in similar positions. I find these kinds of things (a) benefit generally from network effects (i.e. the more people using them the more they are improved) and (b) have a tendency to bitrot if they aren’t being used, since they aren’t otherwise a part of the build or test process.
I guess what I’m saying is: I’m happy to add this as-is, since I don’t think there’s much harm in it. But there’s probably a broader conversation to be had about what individual people’s workflows are like so we can maybe get more people using some more unified dev tooling like this.
|
Review app for commit 7bd3021 deployed to Heroku: https://hge-ci-pull-2547.herokuapp.com |
|
Thanks for the review @lexi-lambda ! Would you mind running both I definitely understand your concerns above. The main motivators here were:
re. bash: I think there's little complexity here that isn't inherent to setting up a basically functioning environment and dev workflow. But i basically agree, and certainly the script will get more complex. re. 3: yeah I fully expect to be fixing this when stuff breaks it :P . If it gets some buy-in I would like to add it to the tests, possibly replacing some existing scripts. I certainly wouldn't want to force other folks to change their workflow, although I'd love to benefit from the knowledge other people have if this script can be adapted/improved to e.g. support things you like about your workflow. |
|
Review app for commit 66a5119 deployed to Heroku: https://hge-ci-pull-2547.herokuapp.com |
At the moment we can...
...run tests in isolation, generating coverage report:
$ dev.sh test
You can pass args to pytest as well. e.g. to run a specific test:
$ dev.sh test -k "test_jsonb_has_all"
Launch a postgres container with useful dev defaults, with PostGIS,
cleaning up afterwards:
$ dev.sh postgres
Build and launch graphql-engine in dev mode, connecting with a
`postgres` launched above
$ dev.sh graphql-engine
These are assets forked from https://github.com/tibbe/ekg at revision fe5e5a1e67 Only monitor.js was modified slightly to take a local port from the URL fragment. I think we should feel free to continue modifying this, or replace with our own tooling later on.
66a5119 to
24409c7
Compare
|
Review app for commit 24409c7 deployed to Heroku: https://hge-ci-pull-2547.herokuapp.com |
I found this default a super annoying time waster.
|
Review app for commit 3e2e143 deployed to Heroku: https://hge-ci-pull-2547.herokuapp.com |
lexi-lambda
left a comment
There was a problem hiding this comment.
This LGTM. My previous general comments still stand, but the implementation looks good as far as I can tell, and it all seems to work fine on my (macOS) machine.
|
Thanks, I guess it needs approval from those three other folks? Merge without squashing please |
|
Review app for commit a5a0763 deployed to Heroku: https://hge-ci-pull-2547.herokuapp.com |
|
Review app https://hge-ci-pull-2547.herokuapp.com is deleted |
NOTE: after rebasing on master two tests fail with "x-hasura-json-required-keys" header is expected but not found. @rakeshkky knows more about this
At the moment we can...
...run tests in isolation, generating coverage report:
Launch a postgres container with useful dev defaults, with PostGIS,
cleaning up afterwards:
Build and launch graphql-engine in dev mode, connecting with a
postgreslaunched above