Add support for PassengerPreloadBundler #2233

smortex · 2022-04-04T18:52:46Z

The latest passenger include a new option to load ruby bundles earlier,
working around deployment issues. See:

puppet-community-rangefinder · 2022-04-04T18:52:58Z

apache::mod::passenger is a class

Breaking changes to this file WILL impact these 11 modules (exact match):

Breaking changes to this file MAY impact these 3 modules (near match):

apache::vhost is a type

Breaking changes to this file WILL impact these 128 modules (exact match):

Breaking changes to this file MAY impact these 35 modules (near match):

This module is declared in 174 of 579 indexed public `Puppetfiles`.

These results were generated with Rangefinder, a tool that helps predict the downstream impact of breaking changes to elements used in Puppet modules. You can run this on the command line to get a full report.

Exact matches are those that we can positively identify via namespace and the declaring modules' metadata. Non-namespaced items, such as Puppet 3.x functions, will always be reported as near matches only.

smortex · 2022-04-04T22:20:42Z

@chelnak not sure about what "community-labeller" is supposed to check (failed test in first commit)… I labelled the issue and re-run the test but the outcome was the same 🤷 Then I saw CI was broken, so I added a commit to fix this and that "community-labeller" issue did not trigger. I see you authored this GHA maybe you are interested in this info?

ekohl · 2022-04-04T22:25:22Z

@smortex I think #2232 is supposed to fix it. If this commit is included in your PR then I think we can conclude #2232 didn't fix it.

chelnak · 2022-04-05T08:13:36Z

That's the one @ekohl !

@smortex It turns out getting the right scope for this sort of thing is harder than I thought. However it should be fixed in the commit mentioned above.

Thanks for pointing it out 👍🙂

david22swan

Other than the below, this look's good to me.

manifests/mod/passenger.pp

The latest passenger include a new option to load ruby bundles earlier, working around deployment issues. See: * phusion/passenger#2410 * phusion/passenger#2409

bastelfreak · 2022-04-06T07:59:43Z

spec/acceptance/mod_php_spec.rb

@@ -44,7 +44,7 @@ class { 'apache::mod::php': }
      describe file("#{apache_hash['mod_dir']}/php7.4.conf") do
        it { is_expected.to contain 'DirectoryIndex index.php' }
      end
-    elsif os[:family] == 'redhat' && os[:release] =~ %r{^8\.}
+    elsif os[:family] == 'redhat' && os[:release] =~ %r{^8($|\.)}


looks a bit complicated. %r{^8} should be fine?

Quite agree, at the current rate of RedHat releases I think we can ignore the issues that might arise the release following RedHat 79 😁 …

/^8\b/ should also do it as expected

CentOS Stream verion is just `8`, not `8.foo`.

david22swan

LGTM

bastelfreak · 2022-04-06T09:10:07Z

manifests/mod/passenger.pp

@@ -343,6 +346,7 @@
  $passenger_installed_version                                                               = undef,
  $passenger_instance_registry_dir                                                           = undef,
  $passenger_load_shell_envvars                                                              = undef,
+  Optional[Boolean] $passenger_preload_bundler                                               = undef,


Wouldn't false be the better default?

Suggested change

Optional[Boolean] $passenger_preload_bundler = undef,

Boolean $passenger_preload_bundler = false,

No. Right now undef means "don't output it at all" and the boolean translates to On/Off. This is the correct behavior IMHO, especially since older Passenger versions don't support this so it'd fail on an unexpected statement.

ekohl

Not sure what's up with the CLA check.

david22swan · 2022-04-06T16:40:22Z

@smortex Thanks for putting the work in :)

As for the CLA test, they do that sometimes

smortex requested a review from a team as a code owner April 4, 2022 18:52

smortex added the community label Apr 4, 2022

smortex force-pushed the passenger_preload_bundler branch from 8b1ebfe to 6395913 Compare April 4, 2022 20:06

smortex marked this pull request as draft April 4, 2022 20:08

smortex force-pushed the passenger_preload_bundler branch from 6395913 to d3c21bf Compare April 4, 2022 20:19

smortex marked this pull request as ready for review April 4, 2022 22:20

smortex force-pushed the passenger_preload_bundler branch 2 times, most recently from 08ccf42 to af8094b Compare April 4, 2022 23:00

smortex mentioned this pull request Apr 5, 2022

Pdksync gh cat 8/centos stream8 #2230

Closed

david22swan added the feature label Apr 5, 2022

david22swan requested changes Apr 5, 2022

View reviewed changes

manifests/mod/passenger.pp Outdated Show resolved Hide resolved

Add support for PassengerPreloadBundler

46778b2

The latest passenger include a new option to load ruby bundles earlier, working around deployment issues. See: * phusion/passenger#2410 * phusion/passenger#2409

smortex force-pushed the passenger_preload_bundler branch from af8094b to 75033e3 Compare April 6, 2022 07:57

bastelfreak reviewed Apr 6, 2022

View reviewed changes

Fix CI

2e059c1

CentOS Stream verion is just `8`, not `8.foo`.

smortex force-pushed the passenger_preload_bundler branch from 75033e3 to 2e059c1 Compare April 6, 2022 08:04

smortex requested review from bastelfreak and david22swan April 6, 2022 09:02

david22swan approved these changes Apr 6, 2022

View reviewed changes

bastelfreak reviewed Apr 6, 2022

View reviewed changes

ekohl approved these changes Apr 6, 2022

View reviewed changes

david22swan merged commit 6a699ef into puppetlabs:main Apr 6, 2022

smortex deleted the passenger_preload_bundler branch April 6, 2022 17:27

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Add support for PassengerPreloadBundler #2233

Add support for PassengerPreloadBundler #2233

smortex commented Apr 4, 2022

puppet-community-rangefinder bot commented Apr 4, 2022

smortex commented Apr 4, 2022

ekohl commented Apr 4, 2022

chelnak commented Apr 5, 2022

david22swan left a comment •

edited

Loading

bastelfreak Apr 6, 2022

smortex Apr 6, 2022

smortex Apr 6, 2022

david22swan left a comment

bastelfreak Apr 6, 2022

ekohl Apr 6, 2022

ekohl left a comment

david22swan commented Apr 6, 2022

	Optional[Boolean] $passenger_preload_bundler = undef,
	Boolean $passenger_preload_bundler = false,

Add support for PassengerPreloadBundler #2233

Add support for PassengerPreloadBundler #2233

Conversation

smortex commented Apr 4, 2022

puppet-community-rangefinder bot commented Apr 4, 2022

apache::mod::passenger is a class

apache::vhost is a type

This module is declared in 174 of 579 indexed public Puppetfiles.

smortex commented Apr 4, 2022

ekohl commented Apr 4, 2022

chelnak commented Apr 5, 2022

david22swan left a comment • edited Loading

Choose a reason for hiding this comment

bastelfreak Apr 6, 2022

Choose a reason for hiding this comment

smortex Apr 6, 2022

Choose a reason for hiding this comment

smortex Apr 6, 2022

Choose a reason for hiding this comment

david22swan left a comment

Choose a reason for hiding this comment

bastelfreak Apr 6, 2022

Choose a reason for hiding this comment

ekohl Apr 6, 2022

Choose a reason for hiding this comment

ekohl left a comment

Choose a reason for hiding this comment

david22swan commented Apr 6, 2022

This module is declared in 174 of 579 indexed public `Puppetfiles`.

david22swan left a comment •

edited

Loading