BallAerospace / COSMOS

Ball Aerospace COSMOS
https://ballaerospace.github.io/cosmos-website/
Other
360 stars 129 forks source link

Fix model initialization to pass needs_dependencies to correctly prop… #1736

Closed finchlt closed 2 years ago

finchlt commented 2 years ago

This pull request is a fix for #1731. Each of the updated models is initialized in plugin_model.rb at the line below

https://github.com/BallAerospace/COSMOS/blob/7e14868c27f51022479fb8ad87d3d1f9c12de35b/cosmos/lib/cosmos/models/plugin_model.rb#L207

For each of the models updated the needs_dependencies flag was not propagated to the underlying model initialization call. This results in the dependencies added with the needs_dependenices flag, including but not limited to a plugin level lib folder to be not included in the $LOAD_PATH for the microservices when they are launched. This in tern results in the require errors as documented in #1731.

codecov[bot] commented 2 years ago

Codecov Report

Merging #1736 (529ac53) into master (7e14868) will not change coverage. The diff coverage is n/a.

@@           Coverage Diff           @@
##           master    #1736   +/-   ##
=======================================
  Coverage   78.16%   78.16%           
=======================================
  Files         236      236           
  Lines       18247    18247           
=======================================
  Hits        14263    14263           
  Misses       3984     3984           

Continue to review full report at Codecov.

Legend - Click here to learn more Δ = absolute <relative> (impact), ø = not affected, ? = missing data Powered by Codecov. Last update 7e14868...529ac53. Read the comment docs.

ryanmelt commented 2 years ago

Hey Logan, Thanks for the fix. Is it working for you after making these changes?

finchlt commented 2 years ago

@ryanmelt Yeah its working as I expected after these changes. An important note is that I have only tested that the plugin level lib folder is respected in the following spots.

Have not tested:

jmthomas commented 2 years ago

Note: this appears to work for 'require' but not 'load_utility'. So you can require a file but not instrument it. @finchlt can you confirm?

finchlt commented 2 years ago

@jmthomas I will take a look.

I don't have experience with the load_utility. Are you refertign to this functionality in the api_shared.rb

https://github.com/BallAerospace/COSMOS/blob/7e14868c27f51022479fb8ad87d3d1f9c12de35b/cosmos/lib/cosmos/script/api_shared.rb#L484

This would then map to the case where you used a load_utility call in a script from script runner?

finchlt commented 2 years ago

@jmthomas I was able generated an instrumented script from tools/scriptrunner -> SCRIPT -> View Instrumented Script that could in turn require a module from the plugin lib folder. I tested some basic functionality from that module as well by calling some code inside that module just to check and make sure everything worked.

finchlt commented 2 years ago

@jmthomas Was also able to run the load_utility method in api_shared.rb directly in another script from script runner.

load_utility("<path_to_my_script_that_requires_file_in_plugin_lib_folder>")

Not sure if this hits at your question better.

jmthomas commented 2 years ago

Thanks @finchlt that's what I was looking for. I'll go ahead and merge.