[WIP] Add publish to heroku support #104
Conversation
Rather gross, but proves that it works.
|
A first basic stab at making this work, just to prove the approach. Right now this requires a Heroku CLI plugin, which seems pretty unreasonable. I think this can be replaced with direct API calls, which could clean up a lot of things. But I wanted to prove it worked first, and it does. |
datasette/cli.py
Outdated
| call('now') | ||
|
|
||
| elif publisher == 'heroku': | ||
| # FIXME: need to verify we have heroku, heroku-builds, and are logged in (ugh) |
There was a problem hiding this comment.
Provided the heroku commands output something sensible and quits if you aren't logged in I think that would be fine.
There was a problem hiding this comment.
I'll have to test that and see.
datasette/cli.py
Outdated
|
|
||
| create_output = check_output(['heroku', 'apps:create', '--json']) | ||
| app_name = json.loads(create_output)["name"] | ||
| call(["heroku", "builds:create", "-a", app_name]) |
There was a problem hiding this comment.
Could we do an old-fashioned git init . && heroku create && git push heroku master here instead?
There was a problem hiding this comment.
Yeah - do we want to force git to be installed, though?
There was a problem hiding this comment.
Huh good question. Datasette doesn't work on Windows (a Sanic restriction) and git ships with OS X these days so I don't see it as a particularly nasty limitation. We can show an error if someone tries to run this and they don't have git.
There was a problem hiding this comment.
So, having messed with this a bit, there might be a good reason to stick to the builds API: doing the git approach is super-messy when it comes to updating the app. Either you have to git push --force each time, which eliminates history in the remote, or you have to get super-clever about cloning a remote repo first if the app's already been pushed before in the past.
So I see 3 options:
-
Use the git approach anyway, and just deal with complexity. Upside: no additional plugins or API calls. Downside: messy code, lots of conditionals, potentially slow cloning existing apps.
-
Use heroku-builds, and just prompt for (or auto-install?) the plugin. Upside: simple code. Downside: plugin, potentially brittle there.
-
Call the build API directly (using requests or whatever). Upside: no other binaries needed (would even work w/o the Heroku CLI being installed if we wanted to jump that hoop). Downside: lots more code in datasette to deal with that complexity.
Personally, I'm leaning towards option 2: keep using the builds plugin, and just offer to install it for folks who don't have it.
@simonw how'd you like to proceed?
There was a problem hiding this comment.
I'm happy with option 2 then - heroku plug-ins are pretty painless.
I've not had to think about updates for the Zeit Now and docker package options because they are both meant to be immutable containers that you replace entirely every time you ship code.
I have been thinking about adding an option that lets you "persist" the temporary directory created by "datasette now publish" in case you want to manually customize it or keep it around - something like this:
datasette publish now mydatabase.db --persist-directory=~/my-project
|
any reason I shouldn't land this? |
|
I'd like to do a bit of cleanup, and some error checking in case heroku/heroku-builds isn't installed. |
|
@simonw ready for a review and merge if you want. There's still some nasty duplicated code in cli.py and utils.py, which is just going to get worse if/when we start adding any other deploy targets (and I want to do one for cloud.gov, at least). I think there's an opportunity for some refactoring here. I'm happy to do that now as part of this PR, or if you merge this first I'll do it in a different one. |
|
Actually hang on, don't merge - there are some bugs that #141 masked when I tested this out elsewhere. |
|
OK, now this should work. |
Refs #90