Fixes #15114 - initialize Katello plugin sooner #6059
Conversation
iNecas
force-pushed
the
sooner-initialization
branch
from
080b298
to
efed000
Compare
theforeman-bot
added
Needs testing
Waiting on contributor
and removed
Waiting on contributor
labels
|
Dear @theforeman-bot, it's nice you prefer short lines but I really think it's good idea to give some external links to clarify the patch and it's not a good idea to wrap the links to fit the 72 chars limit |
|
Works as expected, didn't encounter any issues when testing it with theforeman/foreman-tasks#185 |
|
If you look at the failure from the assets precompile test, you will see why we had to make the change this is reverting: http://ci.theforeman.org/job/test_katello_assets_precompile/3069/ruby=2.2/consoleFull |
iNecas
force-pushed
the
sooner-initialization
branch
from
efed000
to
acbf915
Compare
|
There were the following issues with the commit message:
If you don't have a ticket number, please create an issue in Redmine, selecting the appropriate project. More guidelines are available in Coding Standards or on the Foreman wiki. This message was auto-generated by Foreman's prprocessor |
theforeman-bot
added
Needs testing
Waiting on contributor
and removed
Waiting on contributor
labels
|
@ehelms funny thing there was also an issue when the initializer was too low int he engine file. I've put it up a bit and things seemed to start working in the rake task as well. |
|
@iNecas starting to think we need a more robust, predictable system for loading up our plugins :) |
|
@ehelms :) Anyway, it's all green now and works properly with foreman-tasks now. |
|
I'm not usually a fan of comments, however, I think in this instance a comment here about why that needs to be where it is would be quite useful |
iNecas
force-pushed
the
sooner-initialization
branch
from
acbf915
to
469d6b1
Compare
|
There were the following issues with the commit message:
If you don't have a ticket number, please create an issue in Redmine, selecting the appropriate project. More guidelines are available in Coding Standards or on the Foreman wiki. This message was auto-generated by Foreman's prprocessor |
theforeman-bot
added
Needs testing
Waiting on contributor
and removed
Waiting on contributor
labels
|
@ehelms +1 something like this? |
| @@ -87,6 +87,13 @@ def find_assets(args = {}) | |||
| require 'katello/apipie/validators' | |||
| end | |||
|
|
|||
| # make sure the Katello plugin is initialized before `after_initialize` | |||
| # hook so that the resumed Dynflow tasks can rely on everything ready. | |||
| initializer 'katello.register_plugin', :before => :finisher_hook do | |||
There was a problem hiding this comment.
One more thought, we need this to run before foreman_tasks.initialize_dynflow initializer is run? If so, this might could be done more explicitly via:
initializer 'katello.register_plugin', :before => 'foreman_tasks.initialize_dynflow'
There was a problem hiding this comment.
In fact, in theforeman/foreman-tasks#185, the initialization of dynflow runtime was moved to after_initialize - not the dynflow configuration, that can be performed in any initializer block as before, so I've replaced the occurrences of :before => 'foreman_tasks.initialize_dynflow' with :before => :finisher_hook.
Initializing the Katello plugin too late causes issues when resuming Dynflow tasks (for example the loggers not being initialized) Foreman recommends before :finisher_hook as alternative hook to be used https://github.com/theforeman/foreman/blob/1.11.0/config/initializers/assets_paths.rb#L17 Blocks theforeman/foreman-tasks#185
iNecas
force-pushed
the
sooner-initialization
branch
from
469d6b1
to
bfcef57
Compare
|
There were the following issues with the commit message:
If you don't have a ticket number, please create an issue in Redmine, selecting the appropriate project. More guidelines are available in Coding Standards or on the Foreman wiki. This message was auto-generated by Foreman's prprocessor |
| @@ -105,7 +112,7 @@ def find_assets(args = {}) | |||
| end | |||
| end | |||
|
|
|||
| initializer "katello.initialize_cp_listener", after: "foreman_tasks.initialize_dynflow" do | |||
There was a problem hiding this comment.
The :after => "foreman_tasks.initialize_dynflow" condition was not needed with recent foreman_tasks changes and on_init block.
|
Thanks @iNecas for indulging all my questions! ACK |
|
@iNecas This seems to affect rake db:migrate in production. With this change foreman-tasks shows down for our backend services checks. If I revert engine.rb to the previous version, it all works fine again. I filed http://projects.theforeman.org/issues/15220 for the issue Any ideas why this might be happening? |
Initializing the Katello plugin too late causes issues when resuming Dynflow tasks (for example the loggers not being initialized) Foreman recommends before :finisher_hook as alternative hook to be used https://github.com/theforeman/foreman/blob/1.11.0/config/initializers/assets_paths.rb#L17 Blocks theforeman/foreman-tasks#185 (cherry picked from commit 28cb3ef)
|
I've updated dependency on newer foreman-tasks as I forgot to do that initially, sry about the confusion #6088 |
Initializing the Katello plugin too late causes issues when resuming Dynflow tasks (for example the loggers not being initialized) Foreman recommends before :finisher_hook as alternative hook to be used https://github.com/theforeman/foreman/blob/1.11.0/config/initializers/assets_paths.rb#L17 Blocks theforeman/foreman-tasks#185 (cherry picked from commit 28cb3ef)
Initializing the Katello plugin too late causes issues when resuming Dynflow
tasks (for example the loggers not being initialized)
Foreman recommends before :finisher_hook as alternative hook to be used
https://github.com/theforeman/foreman/blob/1.11.0/config/initializers/assets_paths.rb#L17
Blocks theforeman/foreman-tasks#185