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

Rename default extension.json to extension.config #124

Closed
mathetake opened this issue Mar 18, 2021 · 4 comments
Closed

Rename default extension.json to extension.config #124

mathetake opened this issue Mar 18, 2021 · 4 comments
Labels
enhancement New feature or request

Comments

@mathetake
Copy link
Member

mathetake commented Mar 18, 2021

While writing TinyGo tutorial, I realized that we should not limit extension configuration to be json, also the name extension.json sounds ambiguous to me because it does not have a configuration-ish word in its name.

So I would like to rename it to be extension.config or something similar which does not indicate the format and have config-ish meaning in its name. Ideas? @yskopets @nullpo-head

As a workaround for the format, we can add data/example/init/templates/envoy..../${format}/* which contains extension.${format} for every template, but I think that's too much just for resolving this.

@mathetake mathetake added the enhancement New feature or request label Mar 18, 2021
@yskopets
Copy link
Member

@mathetake I can try to explain motivation behind the current solution (and naming).

Envoy configuration model allows developers of Wasm extensions to choose any format for their own configuration.

That's why getenvoy allows developers to do the same - we accept configuration from files named:

  • extension.yaml
  • extension.json
  • extension

See https://github.com/tetratelabs/getenvoy/blob/master/pkg/extension/workspace/model/example.go#L85

We could add extension.config to that list. Or extension.txt. Or something else.

The reason why we always generate extension.json is to steer extension developers in the right direction. We want ecosystem of Wasm extensions to be consistent. Unless there is a very good reason not to use JSON, developers should stick to JSON by default.

In the case of (Tiny)Go, I would still generate extension.json by default. And explain in the Getting Started that a developer can:

  • either add support for JSON (this is always a preferred way)
  • or rename configuration file into extension (or extension.config, or extension.txt, if we add support for those names)

Regarding the name extension.json, it happens that file with Envoy bootstrap config is commonly called envoy.yaml.

So, my approach to naming was "if a file with Envoy bootstrap configuration is called envoy.yaml, then it's logical to call file with extension configuration extension.yaml/extension.json".

I'm open for changes.

@mathetake
Copy link
Member Author

Thanks Yaro for explaining the motivation behind the current convention! That makes sense.

We want ecosystem of Wasm extensions to be consistent. Unless there is a very good reason not to use JSON, developers should stick to JSON by default.

One thing I would like to note is that I believe not being able to use json as a format is totally a good reason for TinyGo (and AssemblyScript as well?) since they even don't support the ready-to-use library as you know. Also, I know some people use base64-encoded protobuf in the configuration with C++ SDK. Using protobuf as the format is also good for the control plane side since they can use the library as it is, not hand-crafting json schema.

As a future plan, we would have more languages in getenvoy (@lizan right?), so I think that makes sense to rename to non-json bundled one.

So, my approach to naming was "if a file with Envoy bootstrap configuration is called envoy.yaml, then it's logical to call file with extension configuration extension.yaml/extension.json

Now I understand the motivation why you call just extension.json Thanks.

So I would suggest using extension or extension.config (both indicating any format) as a default name.

@mathetake
Copy link
Member Author

I'm OK with renaming the file (in the generation phase) to something like that only for TinyGo. But I have some concerns that would make the getenvoy's code needlessly complicated. WDYT?

@yskopets
Copy link
Member

If you want to rename this file for (Tiny)Go, please go ahead. No concerns about complexity.

My personal preference would be extension.txt or extension.env. Then extension.config.

Let's update the source code of the init template(s) to support comment lines in the config file.

And then include a comment line into extension.txtextension.env / extension.config explaining why we use a non-JSON format.

You will also need to update examples' README.md files, e.g. https://github.com/tetratelabs/getenvoy/blob/master/data/example/init/templates/envoy.access_loggers/default/README.md

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

No branches or pull requests

2 participants