Skip to content
This repository has been archived by the owner on Dec 11, 2024. It is now read-only.

Declarative dependencies #85

Merged
merged 44 commits into from
Oct 8, 2024
Merged

Declarative dependencies #85

merged 44 commits into from
Oct 8, 2024

Conversation

noise64
Copy link
Contributor

@noise64 noise64 commented Sep 24, 2024

Implemented:

  • commands:
    • wasm-rpc declarative pre-build [-c xxx.yaml -c yyy.yaml]: executes "stub build" for are dependency and "add stub deps" for all components
    • wasm-rpc declarative post-build [-c xxx.yaml -c yyy.yaml]: executes "compose" for all components (uses copy if no deps were )
  • manifest resolution:
    • if they are passed directly to the command, then that explicit set of manifests are used
    • if no manifests are specified as command line arg, then we search for golem.yaml in the current and parent folders, and use the one closest to the root, then we "pre-parse" this golem.yaml, which can define the include property (see below in examples), which will be used as a glob expression for finding more manifests
  • manifest model:
    • a generic open application model
    • a specific one extracted from the above with out custom component properties and traits:
      • component types
        • wasm: we can extended this in golem-cli with e.g. durable and other deployment relevant properties
        • wasm-build: for customizing generic build properties, currently only contains the build dir and the include glob
        • wasm-rpc-stub-build: for customizing the stub generation and build, can be specific for a stub by component name, when no component name is specified, then it used as a common preset
    • both models should be reusable and extendable in golem-cli
    • path properties are resolved relative to the yaml source which contains them
  • validations:
    • the manifest are validated in a mostly non-short-circuiting manner, and the validation result can contain warns and errors, also the warns and errors contain context (e.g. source manifest file and component name)
    • some examples for validations:
      • warns for extra and unknown properties in our custom properties and traits
      • uniqueness checks, e.g.: errors for duplicated component names
      • cross checks, e.g.: errors for unknown referenced component names
      • warns for duplicated dependencies
  • added some coloring to build messages (similar in colors to cargo, but no padding on actions)
  • changed files copies to always change modtime
  • component build commands with optional inputs and outputs
  • up-to-date checks (WIP)
  • replaced wasm-compose with wac-graph plugs, and now non-used stubs are skipped

Example specification:

apiVersion: core.oam.dev/v1beta1
kind: Application
metadata:
  name: App
spec:
  # All of these can be in different yaml sources
  components:
    - name: common-wasm-build
      type: wasm-build
      properties:
        buildDir: out
        include: src/components/*/golem.yaml

    - name: common-wasm-rpc-stub-build
      type: wasm-rpc-stub-build
      properties:
        alwaysInlineTypes: true

    - name: component-one-stub-build
      type: wasm-rpc-stub-build
      properties:
        componentName: component-one
        buildDir: custom-build-dir
        alwaysInlineTypes: false

    - name: component-one
      type: wasm
      properties:
        build:
          - command: npm run componentize
            inputs:
              - src
              - wit
            outputs:
              - out/comp_a.wasm
        inputWasm: out/build/component-one/component.wasm
        outputWasm: out/components/component-one.wasm
        wit: src/components/component-one/wit
      traits:
        - type: wasm-rpc
          properties:
            componentName: component-two
        - type: wasm-rpc
          properties:
            componentName: component-three
      
    - name: component-two
      type: wasm
      properties:
        inputWasm: out/build/component-two/component.wasm
        outputWasm: out/components/component-two.wasm
        wit: src/components/component-one/wit
      traits:
        - type: wasm-rpc
          properties:
            componentName: component-three

Some screenshots about coloring and validation:
image
image
image

TODO:

  • recheck "add stub dep" up-to-date-check

TODO In follow up PR:

  • more validation tests
  • build tests

wasm: out/component.wasm
composedWasm: out/component-composed.wasm
traits:
- type: worker-rpc
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
- type: worker-rpc
- type: wasm-rpc

Copy link
Contributor Author

Choose a reason for hiding this comment

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

renamed

- type: worker-rpc
properties:
componentName: component-two
- type: worker-rpc
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
- type: worker-rpc
- type: wasm-rpc

Copy link
Contributor Author

Choose a reason for hiding this comment

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

renamed

spec:
components:
- name: component-one
type: component-durable
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
type: component-durable
type: durable

I'm assuming this would be durable or ephemeral (no need, I think, to replicate the component part since it's already in the components part of the spec.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

since we won't really support anything else (at least for now) we can make them shorter, but if we imagine that we might have different kind of components (e.g. not workers), then we might want to think more about this. note that here the OAM component and our component naming kinda works out now, but the OAM component is something more general. but for now probably we should go with your recommendation.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

now that i'm thinking about it, in our model durable and ephemeral are the property of a component version, and not a component, so maybe we use simply wasm?

or is it okay that in these files it always references the latest one?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

renamed, and kept as a component type for now

properties:
wit: wit
wasm: out/component.wasm
composedWasm: out/component-composed.wasm
Copy link
Contributor

@jdegoes jdegoes Sep 24, 2024

Choose a reason for hiding this comment

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

I think we should stay away from 'composed' terminology as much as possible. A user may think in terms of 'output' or 'target', but probably not 'composed'.

Suggested change
composedWasm: out/component-composed.wasm
output-wasm: out/component-final.wasm

Copy link
Contributor Author

Choose a reason for hiding this comment

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

renamed, but with using camelCase

type: component-durable
properties:
wit: wit
wasm: out/component.wasm
Copy link
Contributor

Choose a reason for hiding this comment

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

Could this legitimately be called input-wasm???

Suggested change
wasm: out/component.wasm
input-wasm: out/component.wasm

Copy link
Contributor Author

@noise64 noise64 Sep 24, 2024

Choose a reason for hiding this comment

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

Other parts of the schema follow camelCase.

I do agree on wasm and composedWasm not being good names, i'm just afraid that using input and output can be also confusing, given the input is the output of the build step, and e.g. in golem-cli, if we reach the point of not having to compose on the client side, we would have an input wasm alone, or if we would have some other tooling integration, this would be confusing. Given currently this is the property of the component, and not the dependency trait itself - where the input would totally make sense - i'm a bit unsure on this naming. (And we probably do not want to move these to the deps trait, because then it would be redundant when having multiple dependencies)

Copy link
Contributor Author

Choose a reason for hiding this comment

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

but happy to change it for now, and let's how it feels

Copy link
Contributor Author

Choose a reason for hiding this comment

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

renamed, but with using camelCase

@vigoo
Copy link
Contributor

vigoo commented Oct 1, 2024

One question, based on the description only:
Why do we have a type: durable | ephemeral property? It does not affect the stub generator at all - can we only add this property in golem(-cli) where it has an actual meaning and use?

@@ -40,6 +41,12 @@ pub enum Command {
/// Initializes a Golem-specific cargo-make configuration in a Cargo workspace for automatically
/// generating stubs and composing results.
InitializeWorkspace(InitializeWorkspaceArgs),
/// TODO
#[cfg(feature = "unstable-dec-dep")]
Declarative {
Copy link
Contributor

Choose a reason for hiding this comment

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

I hope this won't be the final name of the command :)

Copy link
Contributor Author

Choose a reason for hiding this comment

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

no, this is just the WIP name while it is still behind a feature

commands::dependencies::add_stub_dependency(
&app.stub_wit(dep_component_name),
&app.component_wit(component_name),
true, // TODO: is overwrite ever not needed?
Copy link
Contributor

Choose a reason for hiding this comment

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

I added it to protect the potentially user-written files from getting overwritten. I think in the declarative mode, especially if we do the next planned step which moves the user-defined part "inside", we can assume that all wit directories are managed by this tool so it can freely modify everything except the original user defined wit specs.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Added a note about always using true for this in declarative mode

@noise64
Copy link
Contributor Author

noise64 commented Oct 1, 2024

@vigoo for the ephemeral / durable: if it's okay to have incompatible yaml formats, the we can make it different here (as component-type is mandatory in this model)

Another option could be using "component" or "golem" as component type, and only add durability as a property in golem-cli. That way it would compatible and probably even easier to extend in golem-cli.

@noise64
Copy link
Contributor Author

noise64 commented Oct 2, 2024

After discussing with @vigoo:

  • removed durable and ephemeral (can be added as an extra prop in golem-cli)
  • unified naming on traits and oam components to use wasm, wasm-build, wasm-rpc, wasm-rpc-stub-build

@noise64 noise64 marked this pull request as ready for review October 3, 2024 12:41
@noise64 noise64 merged commit 9785eae into main Oct 8, 2024
2 checks passed
@noise64 noise64 deleted the dec-dep branch October 8, 2024 06:16
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants