Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Added external lib folder to autoload path #3307

Merged
merged 1 commit into from
Jan 25, 2024

Conversation

abujeda
Copy link
Contributor

@abujeda abujeda commented Jan 19, 2024

In IQSS, we have 2 custom BatchConnect templates that require 2 ruby classes in an specific lib folder.
We are currently adding these 2 ruby files into the installed OnDemand lib folder: /var/www/ood/apps/sys/dashboard/lib under the expected path: /ood_core/batch_connect/templates/

We would like support to load these classes from the external config root to keep all customizations and overrides together as well as not modify the dashboard file structure.

We might develop as well custom SmartAttributes and this will help to deploy them

@johrstrom
Copy link
Contributor

Why not use initializers in /etc? I don't think you should be installing files into /var because we overwrite them during install.

Also shouldn't lib/ already be a part of the load path? I think that's how we use current_user.rb to template config files.

@abujeda
Copy link
Contributor Author

abujeda commented Jan 19, 2024

/var/www/ood/apps/sys/dashboard/lib (where CurrentUser is) and /var/www/ood/apps/sys/dashboard/app/lib (where SmartAttributes are) are both in the autoload path, but are part of the OOD installation and we do not want to add files to these locations if possible.

/etc/ood/config/apps/dashboard is used for custom templates and initializers, but the /etc/ood/config/apps/dashboard/lib is not used to load ruby classes. The initializers folder cannot be used because it is not in the path to load classes required from other files. It is used load files in the initializers folder only.

I am not aware of any other location in /etc that is in the OOD load path

@johrstrom
Copy link
Contributor

The initializers folder cannot be used because it is not in the path to load classes required from other files.

I guess I'll have to check this out to see what you mean - it seems to me you can define/redefine anything in an initializer - so defining a new SmartAttribute in an initializer seems doable.

Or is it that you need to load a library, say require 'json' in the SmartAttribute and that library json is not available in the initializer?

@abujeda
Copy link
Contributor Author

abujeda commented Jan 19, 2024

When I tried, any require statement for a class that is located inside initializer will result in class not found error.
What I did to create a custom SmartAttribute was to open the SmartAttributes module

# initializers/smart_attributes.rb
module SmartAttributes
  require 'smart_attributes/attributes/bc_aday'
end

And created the SmartAttribute under: initializers/smart_attributes/attributes/bc_aday.rb
But I got a class not found error.

I did the same with the changes in this PR and adding these files under: /etc/ood/config/apps/dashboard/lib and it works as expected

@johrstrom
Copy link
Contributor

Can you share the smart attribute you are trying to use? I just tried with this as an initializer and the form did recognize the jo_test_bc item in the form.

test_bc.rb.txt

Maybe you needed to wrap it in a Rails.application.config.after_initialize block? They changed the way stuff is loaded during initializers in Rails 6.

@abujeda
Copy link
Contributor Author

abujeda commented Jan 19, 2024

OK - the way you implemented the SmartAttribute will work. All is contained within a single file and by opening the AttributeFactory class the JoTestBc can be loaded without a require.

This way will work and I can adapt my ruby code to be all in one class, but I cannot create multiple ruby classes in multiple files and use regular require to load them if created in the initializers folder. And I wouldn't be able to reference classes created here from lets say a custom template.

@johrstrom
Copy link
Contributor

OK, Let me think on this. On the one hand - yes being able to add any ruby library is powerful - on the other ... that power can cause issues or vulnerabilities.

Let me think on it for a bit and see what we can do.

@johrstrom johrstrom self-requested a review January 25, 2024 14:48
Copy link
Contributor

@johrstrom johrstrom left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ya know, I think I'm coming around to this. I see all sorts of hacks around the same.

That said - I think you can require '/full/path/to/the/file'.

In any case, we can move on this.

@johrstrom johrstrom merged commit ed5f538 into OSC:master Jan 25, 2024
20 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants