Use load when loading static service models

[kbrock]

Before

When development reloads classes, MiqAeService models run into an issue.

1. reference an unknown automate service model.
2. calls MiqAeMethodService.const_missing
3. MiqAeServiceModelBase.create_service_model_from_name
4. MiqAeServiceModelBase.create_service_model
5. if file containing service model is found require file_name HERE
6. reference same service model
7. since we required file, it is found and all is well

If rails reloads a class, it unloads the service models, but since the file is still marked as required, the require does nothing.
So the workflow changes as follows:

5. the file was already required, so requre is a no-op
6. reference same constant (still unknown) - step 1
7. calls MiqAeMethodService.const_missing
8. infinite loop

To display the dialog for provisioning a Vm, automate is called inline in the webserver, so the webserver starts crashing with stack overflow errors.

irb(main):001:0> MiqAeMethodService::MiqAeServiceVm
=> MiqAeMethodService::MiqAeServiceVm
irb(main):002:0> reload!
Reloading...
=> true
irb(main):003:0> MiqAeMethodService::MiqAeServiceVm
/Users/kbrock/.gem/ruby/3.0.6/gems/activesupport-6.1.7.6/lib/active_support/core_ext/string/inflections.rb:87:in `safe_constantize': stack level too deep (SystemStackError)
	from manageiq-automation_engine/lib/miq_automation_engine/engine/miq_ae_method_service/miq_ae_service_model_base.rb:159:in `service_model_name_to_model'
	from manageiq-automation_engine/lib/miq_automation_engine/engine/miq_ae_method_service/miq_ae_service_model_base.rb:117:in `create_service_model_from_name'

After

irb(main):001:0> MiqAeMethodService::MiqAeServiceVm
=> MiqAeMethodService::MiqAeServiceVm
irb(main):002:0> reload!
Reloading...
=> true
irb(main):003:0> MiqAeMethodService::MiqAeServiceVm
=> MiqAeMethodService::MiqAeServiceVm
irb(main):004:0> quit

[Fryguy]

If a model ends up going through a Rails reload, will this be caching an old model? Or does Rails reload handle that?

[kbrock]

good catch. this will cache the old model and can cause problems.

we are caching quite a few classes in this code.

I am not really seeing these used too much, so not sure exactly why they are cached. e.g.: I don't see ar_model used anywhere

The associations are cached, but I think those may be ok.

The load change removes an infinite stacktrace. So that is more obvious when it is fixed, wondering if I will detect these other problems. Will run this code for a while.

[Fryguy]

Has this always been true or is this new as of zeitwerk? I thought the whole point of class reloading is that it would internally re-load the file.

[kbrock]

These are not in a path that reflect the package. This must be confusing the loader.

Do you want me to experiment with moving the files into a path that reflectst he package?

This method would change to:

def self.create_service_model(ar_model)
  unless File.exist?(model_to_file_path(ar_model))
    dynamic_service_model_creation(ar_model, service_model_superclass(ar_model))
  end
end

[kbrock]

So the caching issue is an existing issue. Not sure if it will be an issue but want to wait until I get a reproducer.

The load is the only part I need. I'll change this refactoring to do that.

[kbrock]

update:

• I trimmed down to the minimum change that makes things work locally

We can probably fix this changing the way zeitwerk is implemented, but I was hoping to get this minimal change in first

[miq-bot]

Checked commit kbrock@aea5b21 with ruby 2.7.8, rubocop 1.56.3, haml-lint 0.51.0, and yamllint
1 file checked, 0 offenses detected
Everything looks fine. 👍

[Fryguy]

I'm good with merging this as is to get rid of the stack overflow.

[Fryguy]

Backported to quinteros in commit 5f47ecb.

commit 5f47ecbf645fa57224aad614ae651a901e5b6492
Author: Jason Frey <[email protected]>
Date:   Fri Dec 8 16:02:15 2023 -0500

    Merge pull request #536 from kbrock/const_get
    
    Const get
    
    (cherry picked from commit 9e3f656946fa5995fc77705ef7609363a77b4f48)