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

Better support for running Plone on PaaS providers such as Heroku #39

Merged
merged 2 commits into from
Apr 13, 2018

Conversation

zupo
Copy link
Member

@zupo zupo commented Apr 6, 2018

Some containerized PaaS providers run the build process in a different location than where they run the end product, i.e. the app server.

For example, when I run buildout on Heroku, the resulting zope.conf file contains a line like so:

%define INSTANCEHOME /tmp/build_857f615d508f92191ca58c6947e2def8/parts/instance

This path does not exist in the production environment, as there, the instance home is at /app/parts/instance.

This commit allows me to set the instance home as an option inside the plone.recipe.zope2instance section in my buildout.cfg file:

[instance]
recipe = plone.recipe.zope2instance
instance-home = /app/parts/instance
client-home = /app/var/instance

@zupo
Copy link
Member Author

zupo commented Apr 6, 2018

This PR is required to fix plone/heroku-buildpack-plone#25.

Copy link
Member

@davisagli davisagli left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The new options should be mentioned in the readme and changelog.

values.append(var)
env_vars = zip(keys, values)
cgi_environment_vars = '\n'.join(["%s %s" % (env_var[0], env_var[1])
for env_var in env_vars])
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It looks like this processing could easily be factored into a function to avoid duplicating code between environment_vars and cgi_environment_vars.

@zupo
Copy link
Member Author

zupo commented Apr 9, 2018

@davisagli: I intentionally left out things as I was not sure if the PR should be against a 4.x branch, or rather against master and then I possibly do another PR for backporting?

If I'm looking correctly, travis is broken on 4.x branch, so that made me question things.

@davisagli
Copy link
Member

@zupo depends which release series of Plone you are targeting. The 4.x branch is used by Plone 5.1 and master is used by Plone 5.2 (with Zope 4). I don't think it matters whether you make a PR to master and then backport, or to 4.x and then forward-port.

I just opened #40 to fix the travis build on the 4.x branch.

Some containerized PaaS providers run the build process in a different
location than where they run the end product, i.e. the app server.

For example, when I run buildout on Heroku, the resulting zope.conf file
contains a line like so:

```
%define INSTANCEHOME /tmp/build_857f615d508f92191ca58c6947e2def8/parts/instance
```

This path does not exist in the production environment, as there the instance
home is at `/app/parts/instance`.

This commit allows me to set the instance home as an option inside the
plone.recipe.zope2instance section in my buildout.cfg file:

```
[instance]
recipe = plone.recipe.zope2instance
instance-home = /app/parts/instance
client-home = /app/var/instance
```
This is needed so that we are able to tell Plone to run in HTTPS mode on
PaaS providers such as Heroku. Currently, it is achieved with the following
Ugly Hack™:
https://github.com/plone/heroku-buildpack-plone/blob/f22284cef9d790970d309c8c4591441d24b961ec/configure_zopeconf.py#L22
@zupo zupo changed the title Support setting the instance-home parameter with options Better support for running Plone on PaaS providers such as Heroku Apr 11, 2018
@zupo
Copy link
Member Author

zupo commented Apr 12, 2018

@davisagli: added documentation, tests; removed code duplication. Please review.

Copy link
Member

@davisagli davisagli left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks good to me.

@zupo zupo merged commit 05f774c into plone:4.x Apr 13, 2018
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants