Skip to content
This repository has been archived by the owner on Nov 13, 2023. It is now read-only.

Allow plugins to use the OverridesLoader #21

Closed
ghost opened this issue Sep 12, 2018 · 3 comments · Fixed by #30
Closed

Allow plugins to use the OverridesLoader #21

ghost opened this issue Sep 12, 2018 · 3 comments · Fixed by #30
Labels
enhancement New feature or request plugins This issue involves the Plugin Facility

Comments

@ghost
Copy link

ghost commented Sep 12, 2018

Plugins to a base cli should be able to modify certain aspects of the CLI. They should make use of the current overrides loader so that not much custom work has to be done. Plugins that perform such an action will be referred to as Infrastructure Plugins

For now we will be talking about the CredentialManager section as that is the only overrides available right now.

Install Behavior

  • Installing an infrastructure plugin will modify a settings.json file (to be discussed later)
    • Any overrides provided by the plugin will now be saved in this file, overwriting any previous settings that were there before.
  • Any migration that must occur due to the new plugin will not happen automatically
  • Overrides are provided by the plugin in the imperative config overrides object.

Settings file

  • A settings.json file will be needed to reliably keep track of which infrastructure plugin to use for each overrides.
    • This allows the consumer to choose which plugin gets used, easily, in the field.
    • We are preparing for a possible config group later
  • On each run of the cli this file is loaded so that we can determine what settings are available
    • If it does not exist, it is created with all the defaults specified.
  • It will be stored in the CLI home directory

Proposed Layout of File

Since we are expecting this file to be used for settings in the future and we want to handle any new overrides, I am proposing the following initial file structure.

export interface ISettingsFile {
  overrides: {
    /**
     * If null, use the default provided by the CLI
     * If string, use the option provided by the plugin named in the string
     */
    [K in keyof IImperativeOverrides]: null | string
  };
}

So for now with no plugins the generated file will look like this:

{
  "overrides": {
    "CredentialManager": null
  }
}

This allows for future overrides to be created as needed without any changes to the load/save actions.

Plugin Load Behavior

  • Infrastructure Plugins and Command Plugins (plugins that add a command group) will be the managed by the PMF.
  • The PMF is now the first thing that gets control in Imperative.init
    • Now divided into 2 parts: Loading and Initializing
    • Loading will loop through all plugins in plugin.json.
      • Commands/Groups/Profiles are saved in memory for later injection
      • Imperative config object is returned with the new overrides provided by any plugin. The facility will look in settings.json to determine which one will be used for each overrides.
    • Initializing takes the preloaded Commands/Groups/Profiles and defines them at their current spot in Imperative
  • After plugins are loaded, the config object will contain the correct overrides object. This ties nicely into the current operations of the overrides loader. I don't think much will need to change with that class.

See design docs for the before and after in the next comment:

@ghost ghost added enhancement New feature or request plugins This issue involves the Plugin Facility labels Sep 12, 2018
@ghost
Copy link
Author

ghost commented Sep 12, 2018

Current Init Strategy

infrastructure plugin workflow old

Proposed Init Strategy

infrastructure plugin workflow

@ghost ghost changed the title Allow plugins to make use of the OverridesLoader Allow plugins to use the OverridesLoader Sep 12, 2018
@tucker01
Copy link
Contributor

Just a point of clarification for Settings file. The CLI home directory is dictated by the imperative config defaultHome property (may or may not be .brightside 😄).

@ghost
Copy link
Author

ghost commented Sep 12, 2018

Good point. I will update the issue

@ghost ghost added the in progress label Sep 12, 2018
@ghost ghost self-assigned this Sep 12, 2018
ghost pushed a commit that referenced this issue Sep 14, 2018
Still need to implement the settings concept.

Signed-off-by: Wright, Christopher R <Christopher.Wright@ca.com>
ghost pushed a commit that referenced this issue Sep 14, 2018
Signed-off-by: Wright, Christopher R <Christopher.Wright@ca.com>
ghost pushed a commit that referenced this issue Sep 14, 2018
Signed-off-by: Wright, Christopher R <Christopher.Wright@ca.com>
ghost pushed a commit that referenced this issue Sep 17, 2018
Signed-off-by: Wright, Christopher R <Christopher.Wright@ca.com>
ghost pushed a commit that referenced this issue Sep 17, 2018
Still need to handle a relative path load in a plugin but we got a starting point!

Signed-off-by: Wright, Christopher R <Christopher.Wright@ca.com>
ghost pushed a commit that referenced this issue Sep 17, 2018
This is super ugly but dll managers probably aren't either :/

Signed-off-by: Wright, Christopher R <Christopher.Wright@ca.com>
ghost pushed a commit that referenced this issue Sep 17, 2018
Signed-off-by: Wright, Christopher R <Christopher.Wright@ca.com>
ghost pushed a commit that referenced this issue Sep 18, 2018
Signed-off-by: Wright, Christopher R <Christopher.Wright@ca.com>
ghost pushed a commit that referenced this issue Sep 18, 2018
Signed-off-by: Wright, Christopher R <Christopher.Wright@ca.com>
ghost pushed a commit that referenced this issue Sep 18, 2018
Signed-off-by: Wright, Christopher R <Christopher.Wright@ca.com>
ghost pushed a commit that referenced this issue Sep 19, 2018
We still allow the cli command to be defined though. A hard crash is only encountered when trying to access credentials.

Signed-off-by: Wright, Christopher R <Christopher.Wright@ca.com>
ghost pushed a commit that referenced this issue Sep 19, 2018
Signed-off-by: Wright, Christopher R <Christopher.Wright@ca.com>
ghost pushed a commit that referenced this issue Sep 19, 2018
Signed-off-by: Wright, Christopher R <Christopher.Wright@ca.com>
ghost pushed a commit that referenced this issue Sep 19, 2018
Signed-off-by: Wright, Christopher R <Christopher.Wright@ca.com>
ghost pushed a commit that referenced this issue Sep 20, 2018
Signed-off-by: Wright, Christopher R <Christopher.Wright@ca.com>
ghost pushed a commit that referenced this issue Sep 20, 2018
Signed-off-by: Wright, Christopher R <Christopher.Wright@ca.com>
ghost pushed a commit that referenced this issue Sep 20, 2018
Signed-off-by: Wright, Christopher R <Christopher.Wright@ca.com>
ghost pushed a commit that referenced this issue Sep 20, 2018
Signed-off-by: Wright, Christopher R <Christopher.Wright@ca.com>
ghost pushed a commit that referenced this issue Sep 20, 2018
Signed-off-by: Wright, Christopher R <Christopher.Wright@ca.com>
ghost pushed a commit that referenced this issue Sep 20, 2018
Signed-off-by: Wright, Christopher R <Christopher.Wright@ca.com>
ghost pushed a commit that referenced this issue Sep 20, 2018
Signed-off-by: Wright, Christopher R <Christopher.Wright@ca.com>
ghost pushed a commit that referenced this issue Sep 20, 2018
Signed-off-by: Wright, Christopher R <Christopher.Wright@ca.com>
ghost pushed a commit that referenced this issue Sep 21, 2018
Signed-off-by: Wright, Christopher R <Christopher.Wright@ca.com>
@ghost ghost added the review label Sep 27, 2018
@ghost ghost removed in progress labels Oct 2, 2018
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
enhancement New feature or request plugins This issue involves the Plugin Facility
Projects
None yet
Development

Successfully merging a pull request may close this issue.

1 participant