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

Modules and execution plans #3880

Open
bretg opened this issue Aug 23, 2024 · 1 comment
Open

Modules and execution plans #3880

bretg opened this issue Aug 23, 2024 · 1 comment

Comments

@bretg
Copy link
Contributor

bretg commented Aug 23, 2024

Having more modules is great, but we've found that execution plans are more difficult to manage than they could be.

  • They're quite powerful, but complicated.
  • They're not merged like most other configs - if they exist at both the host level and the account level, the module is run twice.
  • There's no guidance to host companies on how to determine where to put the execution plan.
  • Not all modules support an "enabled" flag to skip the module for accounts that haven't turned it on

So we'd like to propose the following:

  • A general recommendation that modules execution plans are defined at the host level. No code changes -- just guidance.
  • Every module needs to support an enabled flag in its config so that when the module is called on every request. The idea is that modules need to return as quickly as possible when it's not enabled.
  • Documentation should be updated to reflect these guidelines

We discussed having core be aware of the enabled flag, but currently the module config is opaque to PBS-core. It could cause duplicate parsing of config JSON. It would be an upgrade of the module infrastructure to support a core-visible enabled flag.

PBS-Go reads the module config from the merged config, looking for an enabled flag.

We agreed to consider this offline and continue the discussion in this issue.

@bretg
Copy link
Contributor Author

bretg commented Sep 20, 2024

Discussed these options:

  1. Do nothing.
  2. PBS-core could start requiring module config in order to call the module. Even empty config. If such module config exists, the module is considered enabled.
  3. Have core look for an enabled flag and just don't bother calling the module.
  4. Establish a review convention that all modules must support an enabled flag and the first this every module hook does is confirm enablement.

We're leaning heavily towards option 2:

  • Execution plan can be global
  • PBS-core merges account module config with host-level module config, with account-level taking precedence
  • Before actually calling a module hook, PBS-core would look for module config. If no config, no call.
  • If a module has host-level config, it will always be called.
  • Else, check the account-level config -- the module will only be called if that account has an entry, even a null entry is ok.
  • All current open source modules require some kind of config, so this works ok.
  • However, there's an implication that if there are any private modules that don't require config, the host company would need to create an empty module config. That would be a breaking change.
  • So, the proposal is a new host company flag that would kick core into a mode where module config is required. e.g. settings.modules.require-config-to-invoke. Start off with the default of this flag false
  • In the future (4.0?), the default of this config might change. Or not.

We'll bring this up again at next weeks meeting, hopefully for the final time.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
Status: Ready for Dev
Development

No branches or pull requests

1 participant