Run app via Procfile

[kevindew]

This allows the dev environment and e2e tests to be able to run this app in a very similar manner to production by using the procfile.

Also updates startup.sh to run workers as well as web.

[cbaines]

What's this being used for? I know nothing about foreman, other than what I've just read on the project page [1], and one of the first things that says is "Ruby users should take care not to install foreman in their project's Gemfile.". I've got nothing against doing something different, but it would be good to know why?

1: https://github.com/ddollar/foreman

[kevindew]

What's this being used for? I know nothing about foreman, other than what I've just read on the project page [1], and one of the first things that says is "Ruby users should take care not to install foreman in their project's Gemfile.". I've got nothing against doing something different, but it would be good to know why?

No problem. It's used as a means to launch the processes of the application. The comment they have in the readme seems to all trace back to this which all seems tied around problems with ruby version - It seems to be an ongoing point of contention with the project: ddollar/foreman#573, ddollar/foreman#678

I think the alternatives to having it in the Gemfile are all a bit messier. We'd have to manage it as a separate dependency in the Dockerfile (with a gem install line) with similar in startup.sh. There's already precedence in gov.uk too for having it via gemfile (see bundle exec foreman commands). Whereas as part of it you've got the upside of it being an executable like the various other developer related tasks via bundle exec and the same version pinned.

A suggestion from @thomasleese is to put it as a govuk_app_config dependency since it's quite related to unicorn. Another approach that occurred to me is that we can have it defined as require: false so it doesn't actually get included into the running app unless explicitly required or bundle exec is run.

[cbaines]

No problem. It's used as a means to launch the processes of the application. The comment they have in the readme seems to all trace back to this which all seems tied around problems with ruby version - It seems to be an ongoing point of contention with the project: ddollar/foreman#573, ddollar/foreman#678

I think the alternatives to having it in the Gemfile are all a bit messier. We'd have to manage it as a separate dependency in the Dockerfile (with a gem install line) with similar in startup.sh. There's already precedence in gov.uk too for having it via gemfile (see bundle exec foreman commands). Whereas as part of it you've got the upside of it being an executable like the various other developer related tasks via bundle exec and the same version pinned.

A suggestion from @thomasleese is to put it as a govuk_app_config dependency since it's quite related to unicorn. Another approach that occurred to me is that we can have it defined as require: false so it doesn't actually get included into the running app unless explicitly required or bundle exec is run.

So, on the GOV.UK specific downsides, I see:

• Adding foreman to the Gemfile requires a unnecessary restriction on the version of the thor gem, e.g. here it is being downgraded.
• Adding foreman to the Gemfile is unnecessary for running and using the application in general.
  • I know that these patches are using it in the Dockerfile and the startup.sh script, but Content Tagger itself has no use for foreman and I don't think it's even used when deploying GOV.UK through govuk-puppet. I'm sure the space usage is minimal, but this isn't very neat.

Anyway, I have very different standards and opinions regarding best practices for package management, so I'm not going to stand in the way of this change.

[kevindew]

@cbaines Thanks for the approval.

You're correct on both of those downsides. The impact of the thor one is pretty minimal as I believe it's used by rails generator - there's PRs open for thor update on foreman so hopefully that'll go through soon and that downside will be cleared up. As for the application in general, it is not necessary but a convenience layer - in production for running workers we're currently grepping the Procfile to run a command from it, which arguably would be nicer to just be able to run a pre-defined command.

I'm going to have a little ponder over this though so may leave it open for a short while.

[kevindew]

Closed in favour of #654