search

theforeman / foreman_rh_cloud

a plugin to Foreman that generates and uploads reports to the Red Hat cloud
GNU General Public License v3.0
6 stars 32 forks source link

correctly register REX features when booting the app #904

Closed evgeni closed 2 months ago

evgeni commented 2 months ago

In https://github.com/theforeman/foreman_rh_cloud/commit/3e9932693ed6cfd232e486596662414c7c70bbc0 the plugin registration part was moved into app.reloader.to_prepare, but not the REX features part. Given the same (registering REX features) works for Katello, and there it was moved into reloader.to_prepare in https://github.com/Katello/katello/commit/c90a1dc46dcf0991127f1f369f83d00e07b524d5, I did the same here.

The :after => 'foreman_remote_execution.register_plugin' part wasn't necessary in my tests, but seemed more correct to explicitly specify.

ShimShtein commented 2 months ago

This looks much better, but still I can't understand why is it necessary. Also I am not sure what would be the impack on reloading in dev environment (to_prepare runs each time Rails reloads the app).

evgeni commented 2 months ago

Impact in terms of performance? .register essentially does an upsert (https://github.com/theforeman/foreman_remote_execution/blob/d4abcf575b61211c3d0d970479431ff7bdbf4c3f/app/models/remote_execution_feature.rb#L30-L72) so as long as it's not reloading multiple times a second, it should be fine?

ShimShtein commented 2 months ago

Merged, thanks @evgeni!

evgeni commented 2 months ago

I have verified this fixes Luna pipes by running:

ansible-playbook pipelines/install_pipeline.yml -e pipeline_os=centos9-stream -e pipeline_type=luna -e pipeline_version=nightly -e '{"packit_prs":["https://github.com/theforeman/foreman_rh_cloud/pull/904"]}'