cornucopia-rs / cornucopia

Generate type-checked Rust from your PostgreSQL.
Other
835 stars 38 forks source link

Reproducible builds #157

Open jacobsvante opened 2 years ago

jacobsvante commented 2 years ago

Copied from the Discord channel:

I think that changes to src/cornucopia.rs should be reproducible no matter which developer machine it's run on. To achieve this my suggestion is that:

  1. All current subcommands to cornucopia are removed
  2. Most options are removed from cornucopia: this includes --destination, --queries-path, --serialize and --sync.
  3. A default is determined (perhaps what cornucopia schema ./schema.sql currently does)
  4. Cornucopia supports a file called cornucopia.toml which contains all the removed options and new configuration for method=live|schema, schema_files postgresl_url. None are required and file is not required either.
  5. New subcommands to cornucopia will instead be: build (which runs live or schema depending on configuration) and init which creates a cornucopia.toml file.
LouisGariepy commented 2 years ago

Personally, I'm all for this. Config files are not only reproducible, but they are also more legible and scalable as we add more and more options/codegen settings.

One question is: how does this affect the lib API. Do we add a public fn read_config() -> Config function and make the generate_* functions take a Config, or should the API functions try to read the config under the hood.

jacobsvante commented 2 years ago

I don't know the internals of cornucopia (yet!), but from your two suggestions I think that fn read_config() -> Config() sounds like the more straightforward approach. Then you could also add a Config-builder in the future if such a need arises when using the lib programmatically.

Virgiel commented 2 years ago

I am all in for a config file, but I am against removing commands. I think we should have both. When the CLI run, it first checks for a local config file cornucopia.toml (overridable with --config command) and then merge with CLI arguments. This way, we can use CLI options and/or a config file.

We would have:

struct Config {
    ...
}

impl Config {
    pub fn from_args() -> Self { ... }
    pub fn from_file(path: &Path) -> Self { ... }
    pub fn merge(self, other: Self) -> Self { ... }
}
LouisGariepy commented 2 years ago

@Virgiel looks good to me!

jacobsvante commented 2 years ago

but I am against removing commands

What about meeting half way here and keeping the live and schema as subcommands, but deprecating them and instead pointing to build? This way we would have a clear way forward without breaking the current CLI API, and promote reproducible builds along the way.

When the CLI run, it first checks for a local config file cornucopia.toml (overridable with --config command) and then merge with CLI arguments.

Sounds good to me too.

LouisGariepy commented 2 years ago

Why use CLI settings (instead of)/(in addition to) a config file?

  1. You don't want to have a config file. Maybe you feel like a near empty config file feels obtuse and unnecessary., Maybe you don't like introducing "yet another" config just for Cornucopia.
  2. You routinely use multiple different CLI flags, and you don't like having to change the config file each time. Maybe you do this in automated environment and its unpractical to modify the config file.

IMO, point 2. is actually not a problem: we could support "profiles" in the config file, so you could cornucopia build <profile> or something along those lines. The lib API is also available for automated use cases. With this, I don't see the appeal of merging CLI flags with the config file.

For my part I don't see major advantages to either keeping or removing CLI options. I feel like config files encourage sharing and reusing, but as I highlighted above, there are also arguments to be made for CLI settings. Having both is flexible, but slightly redundant (multiple ways to do the same thing) and potentially confusing (not clear to newcomers which way is the "default").

@Virgiel, I'd be grateful if you could provide some insights into why you want both CLI flags and a config file.

This is the kind of issue where community feedback is especially important, so if you're reading this, please let us know where you stand :smile:

jacobsvante commented 2 years ago

@LouisGariepy It's wonderful to see your deep care for the project. Makes the future for this project look very promising 👍

Virgiel commented 2 years ago

I understand the appeal of a configuration file. However, I personally despise the need to have a configuration file for any of my tools that complicates the root directory of my projects, like the situation you have with web dev tools. If I want a repeatable build, I simply add a few lines to the project's command runner like package.json for web, makefile for others or just if you are Rusty 🦀

LouisGariepy commented 1 year ago

@Virgiel Your reasoning is sound, and many other users might also feel the same, so I've come to the conclusion that we should support both CLI and config.