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

Re-evaluate module spec syntax and parsing mechanism #94

Closed
edvald opened this issue May 15, 2018 · 4 comments
Closed

Re-evaluate module spec syntax and parsing mechanism #94

edvald opened this issue May 15, 2018 · 4 comments

Comments

@edvald
Copy link
Collaborator

edvald commented May 15, 2018

(supersedes #90)

I'm bumping into some issues with our module spec API. I'm trying to balance a few things and figure out what degree of control to give plugins, and I worry a bit about our current YAML layout.

For a bit of context, I'd first off (and definitely) like to make one important change: parseModule should no longer return a class, but rather a validatable object with something like the following interface (which can be readily validated on the framework side):

{
  config?: ModuleConfig    // this allows the handler to optionally modify the configuration, e.g. set some default values (which are then re-evaluated and validated by the framework)
  services: ServiceConfig[]    // a list of services defined in the module
  tests: TestConfig[]    // specify which tests the module includes
}

We'd then define all of ModuleConfig, ServiceConfig and TestConfig as non-extendable interfaces, potentially each with a generic metadata object property that plugins can use at will.

This would mean we could completely do away with generics on Module, Service etc. across our framework code and leave it to plugins to keep the metadata properties type-safe, as well as make it more safe overall from poor plugin implementations.

Another implication is that module specs can technically have whatever structure or semantics, and plugins just need to convert the spec into the above structure. For example, using the term services may not be immediately intuitive in some contexts, and in some contexts a single service (in our framework parlance) may be implicit in the module definition.

Which leads to the concern: To what degree should module configurations be strictly defined at the framework level, and how do we ensure separation between framework-native parameters and pluggable ones?

Another example (in addition to the semantics of what "service" means), is how tests are defined at the module level. We currently have a tests key in the native module spec but it includes a command key that really only makes sense for container and generic modules. So we'll need plugins to be able to specify whatever instructions for the test execution that make sense for that plugin. Do we then evaluate the test key for every module but allow plugins to extend that definition? Or do we omit it entirely from the native spec and rather try and push for conventions in how tests are defined (i.e. urging for them to have a similar layout as our internal TestConfig interface)? Same goes for the build spec.

Yet another issue is how we maintain a backward-compatible separation of native keys in module specs and plugin-specified keys. Example - say we decide today that name, description, and type are framework-native fields that we validate, but not test. Some plugins then implement test as part of their spec, aaaaand then we change our minds and want test to be in our native spec. We're gonna need to be smart about this. At the very least we need to make the fixed fields part of an API version on our side, and make sure that we catch plugins using conflicting fields properly (and before using the values).

So, I'm trying to balance with a simple and clean API. It's always possible to go full-on XML and massively overcomplicate to deal with stuff like this, but I'd like to arrive at a good compromise where we deal smoothly with these concerns and keep the DX slick.

What do y'all think?

@edvald
Copy link
Collaborator Author

edvald commented May 15, 2018

Here's a thought on the module specs... Maybe this could be a bit simpler if we think of the spec in terms of delimited keys instead of nested objects. Meaning, the native "v0" (we'll settle on v1 a bit later) spec could be something like:

"name": string
"description": string
"tests[].dependencies": string[]
"tests[].env": PrimitiveMap
"tests[].name": string
"tests[].timeout": number
"build.dependencies": string[]
"variables": PrimitiveMap

// etc...

Then plugins can add to this spec a little more easily, no complicated nested specs etc., and we can catch conflicts on initialisation by having plugins declare their added fields.

Might be a little strange/annoying to deal with maps though... not sure what a good/known syntax would be for that.

@edvald
Copy link
Collaborator Author

edvald commented May 15, 2018

I've actually been pondering doing away with maps overall... YAML starts to look clumsy if you

have:
  a:
    lot:
      of:
        these.

@edvald
Copy link
Collaborator Author

edvald commented May 15, 2018

On reflection, this would pretty much wreak havoc with Typescript... Back to the drawing board.

@edvald
Copy link
Collaborator Author

edvald commented May 16, 2018

Decision made on Slack (reposting for posterity): We will avoid maps of objects in the YAML spec, partly for ease of programming and partly to distinguish clearly between fields that are in the spec and user-defined values. And perhaps aesthetic reasons, although that's debatable. We can however use maps for basic key/value maps where the value is a primitive (for example, a map of environment variables, which would otherwise be clunky).

  # so this...
  environments:
    local:
      providers:
        local-kubernetes:
          context: docker-for-desktop
        local-google-cloud-functions: {}
    dev:
      providers:
        google-app-engine: {}
        google-cloud-functions:
          default-project: garden-hello-world
  # becomes this
  environments:
    - name: local
      providers:
        - name: local-kubernetes
          context: docker-for-desktop
        - name: local-google-cloud-functions
    - name: dev
      providers:
        - name: google-app-engine
        - name: google-cloud-functions
          default-project: garden-hello-world

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

No branches or pull requests

1 participant