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

Add ability to define User Plugins in environmental variable that load at init #662

Closed
wants to merge 3 commits into from

Conversation

ntouran
Copy link
Member

@ntouran ntouran commented May 6, 2022

Description

Fixes #569

Checklist

  • The code is understandable and maintainable to people beyond the author.
  • Tests have been added/updated to verify that the new or changed code works.
  • There is no commented out code in this PR.
  • The commit message follows good practices.
  • All docstrings are still up-to-date with these changes.

If user exposed functionality was added/changed:

  • Documentation added/updated in the doc folder.
  • New or updated dependencies have been added to setup.py.

ntouran added 2 commits May 6, 2022 15:57
One was used in testing and another similar but different
one was used in App init. Now they are consistent
and using the same code. A runtime import was required
to do this in order to avoid a circular import.
This allows users to load importable plugins from
a ARMI_USER_PLUGINS environmental variable.

Using user settings is too hard because the plugins
have to be loaded and locked before settings can be
read, since many settings are defined by plugins.

Closes #569
@ntouran ntouran added the enhancement New feature or request label May 6, 2022
@ntouran ntouran requested a review from john-science May 6, 2022 23:01
Comment on lines +124 to +130
for pluginSpec in userPluginInput.split(","):
names = pluginSpec.split(".")
modPath = ".".join(names[:-1])
clsName = names[-1]
mod = importlib.import_module(modPath)
plugin = getattr(mod, clsName)
self._pm.register(plugin)
Copy link
Member

Choose a reason for hiding this comment

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

  1. I might change this:

names = pluginSpec.split(".")

to this:

names = pluginSpec.strip().split(".")

People are always putting spaces after commas, when they write a list.

  1. Vis-a-vis my dream: why does this logic have to happen before we read in a Case Settings file? It seems like this should be able to happen at run time. Naively, I could be missing something.

@john-science
Copy link
Member

I have opened my own branch to solve this problem.

Please hold off working on this feature, so I can test mine. (I know you're busy right now, so that's probably not going to be a problem. lol)

@john-science
Copy link
Member

john-science commented Jun 28, 2022

I have opened a PR for a similar feature: #723

I believe it is documented and working.

@keckler keckler deleted the userplugin branch November 24, 2022 03:11
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Allow users to provide user/case-specific user plugins as input
2 participants