Skip to content
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

"datasette publish cloudrun" command to publish to Google Cloud Run #434

Merged
merged 8 commits into from May 3, 2019

Conversation

Projects
None yet
3 participants
@rprimet
Copy link
Contributor

commented Apr 17, 2019

This is a very rough draft to start a discussion on a possible datasette cloud run publish plugin (see issue #400).

The main change was to dynamically set the listening port in make_dockerfile to satisfy cloud run's requirements.

This was done by running datasette through sh to get environment variable substitution. Not sure if that's the right approach?

@simonw

This comment has been minimized.

Copy link
Owner

commented Apr 18, 2019

Thanks for looking into this!

To clarify: currently, the Dockerfile that we generate looks something like this:

CMD ["datasette", "serve", "--host", "0.0.0.0", "fixtures.db", "--cors", "--port", "8001"]

Your code here changes that CMD line to look like this instead, in order to set the port based on an environment variable:

CMD ["sh", "-c", "datasette serve --port $PORT ..."]

I wonder if this is the only way to do this?

"about_url": about_url,
},
):
image_id = f"gcr.io/{project}/{name}"

This comment has been minimized.

Copy link
@simonw

simonw Apr 18, 2019

Owner

Datasette still supports Python 3.5 so we can't use format strings just yet.

@simonw simonw self-requested a review Apr 18, 2019

@simonw

This comment has been minimized.

Copy link
Owner

commented Apr 18, 2019

I asked @andrewgodwin about this and he confirmed that if we want to read an environment variable we can't use the CMD [...] syntax in the way that we were using it.

He did suggest that if we're doing CMD ["sh", "-c", "datasette serve --port $PORT ..."] we may as well do this instead:

CMD "datasette serve --port $PORT ..."

We should apply some command-line escaping here - if the user passes --version-note=hello$there to datasette publish we need that $ not to be accidentally evaluated as an environment variable.

It looks like shlex.quote is the right way to do that.

@rprimet rprimet marked this pull request as ready for review Apr 29, 2019

@simonw

This comment has been minimized.

Copy link
Owner

commented May 3, 2019

This is amazing - works an absolute treat. Thank you very much!

@simonw simonw merged commit 75a21fc into simonw:master May 3, 2019

1 check passed

continuous-integration/travis-ci/pr The Travis CI build passed
Details
@eyeseast

This comment has been minimized.

Copy link

commented May 3, 2019

This is exactly what I needed. Thank you.

@simonw

This comment has been minimized.

Copy link
Owner

commented May 3, 2019

Here's my first working deployment: https://datasette-j7hipcg4aq-uc.a.run.app/fixtures-c35b6a5/facetable?_facet_array=tags

I deployed it using this:

datasette publish cloudrun fixtures.db --branch=master

The second time I ran the command I got an error:

ERROR: (gcloud.beta.run.deploy) Deployment endpoint was not found. Perhaps the
provided region was invalid. Set the `run/region` property to a valid region and
retry. Ex: `gcloud config set run/region us-central1`

So I ran the command it suggested and then everything worked:

gcloud config set run/region us-central1
datasette publish cloudrun fixtures.db --branch=master

simonw added a commit that referenced this pull request May 3, 2019

@simonw simonw changed the title [WIP] publish on google cloud run "datasette publish cloudrun" command to publish to Google Cloud Run May 3, 2019

@simonw

This comment has been minimized.

Copy link
Owner

commented May 3, 2019

@rprimet

This comment has been minimized.

Copy link
Contributor Author

commented May 3, 2019

The second time I ran the command I got an error:

ERROR: (gcloud.beta.run.deploy) Deployment endpoint was not found. Perhaps the
provided region was invalid. Set the run/region property to a valid region and
retry. Ex: gcloud config set run/region us-central1

Yes, I was able to reproduce this; I used to get prompted for a run region interactively by the gcloud tool before, but maybe this is changing? (the documentation now assumes run/region is set).

Not sure which course of action is best: making datasette ensure that run/region is set beforehand or wait a bit until the gcloud CLI stabilizes?

@simonw

This comment has been minimized.

Copy link
Owner

commented May 3, 2019

Since there's a useful error message I'm OK with revisiting this in a few weeks to see if they change the CLI tool.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
You can’t perform that action at this time.