Plugin refactor pt2 #71
Conversation
Plugins are now registered and loaded separately, and are now defined as modules with an exported `gardenPlugin` function. BREAKING CHANGE: This includes some changes to the project schema and how it is resolved, as well as how the main `Garden` class is instantiated. The `Garden` class is now called with an environment name, which is then fixed for the session. The env configuration is resolved by merging the specific environment configuration with a global configuration specified on the new `global` key in the project config. The schema for the `providers` key also different - its keys should now match plugin names, and contain configuration for those plugins.
edvald
force-pushed
the
plugin-refactor-pt2
branch
from
43dcb55 to
9cd85c3
Compare
This needed a few changes, but will be super useful as complexity grows in system-level deployments.
| @@ -33,10 +33,9 @@ export const configSchema = Joi.object() | |||
| project: projectSchema, | |||
| }) | |||
| .optionalKeys(["module", "project"]) | |||
| .options({ allowUnknown: true }) | |||
There was a problem hiding this comment.
@edvald This causes an error on the module.image key since it's missing from the baseModuleSchema. Should we:
a) Allow unknown keys on the configSchema?
b) Allow unknown keys on the baseModuleSchema?
c) Add the image key to the baseModuleSchema?
There was a problem hiding this comment.
...I'll go for c) unless there are any objections.
There was a problem hiding this comment.
This has actually been fixed in #75
We shouldn't allow unknown keys there, but rather in the individual sub-schemas. I just missed that one in this PR.
There was a problem hiding this comment.
Gotcha. I'll fix it here though so that we don't merge a broken PR.
There was a problem hiding this comment.
I can also merge #75 in here before merging this into master.
There was a problem hiding this comment.
Ah you already did it, all good.
| return <T>new Module(ctx, config) | ||
| } | ||
| // TODO: find a different way to solve type export issues | ||
| let _serviceConfig: ServiceConfig |
There was a problem hiding this comment.
What's this for? Also noticed it on the npm-package plugin.
There was a problem hiding this comment.
It's this slightly annoying thing in Typescript - if a module exports something that uses a certain type, it needs to be explicitly imported in the module. I couldn't quickly find a better way than this - just importing the type without using it raises linting errors, so I needed to put it in a dummy variable.
Deploy k8s system via sub-Garden
This is a bulky one... From the commit msg:
Plugins are now registered and loaded separately, and are now defined
as modules with an exported
gardenPluginfunction.BREAKING CHANGE:
This includes some changes to the project schema and how it is resolved,
as well as how the main
Gardenclass is instantiated. TheGardenclass is now called with an environment name, which is then fixed for
the session. The env configuration is resolved by merging the specific
environment configuration with a global configuration specified on the
new
globalkey in the project config. The schema for theproviderskey also different - its keys should now match plugin names, and
contain configuration for those plugins.