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

Config file of sea-orm-cli generate entity command #1185

Open
billy1624 opened this issue Nov 3, 2022 · 8 comments · May be fixed by #1814
Open

Config file of sea-orm-cli generate entity command #1185

billy1624 opened this issue Nov 3, 2022 · 8 comments · May be fixed by #1814

Comments

@billy1624
Copy link
Member

Motivation

The options and flags for sea-orm-cli generate entity command is getting more and more. It's time to consider a scalable plan: loading the options from a config file.

Proposed Solutions

Load all options by providing the path to the config file: sea-orm-cli generate entity --config generate-entity.config.json. The config file must be a JSON value.

{
    "compact_format": true,
    "expanded_format": true,
    "include_hidden_tables": true,
    "tables": "my_table",
    "ignore_tables": ["seaql_migrations"],
    "max_connections": 1,
    "output_dir": "out",
    "database_schema": "public",
    "database_url": "postgres://root:root@localhost/database",
    "with_serde": "none",
    "with_copy_enums": true,
    "date_time_crate": "chrono",
    "lib": true
}

Related Discussion

@billy1624
Copy link
Member Author

Comments? @tyt2y3

@tyt2y3
Copy link
Member

tyt2y3 commented Nov 5, 2022

Agree. I think there should be a default path where we load this file from, i.e. somewhere in the output_dir.
This would encourage projects to actually commit this file into the repo, somewhat like ts.config.json

Second, only options related to entity generation can be specified, the following:

    "max_connections": 1,
    "output_dir": "out",
    "database_schema": "public",
    "database_url": "postgres://root:root@localhost/database",

should NOT be included.

@aadi58002
Copy link
Contributor

I would like to work on this issue. I will like to use the https://github.com/mehcode/config-rs lib. Which seem to support multiple types as multiple config file hierarchy.

@tyt2y3
Copy link
Member

tyt2y3 commented Dec 9, 2022

Apparently config-rs is written by one of the SQLx authors

@aadi58002
Copy link
Contributor

Nice fact

@aadi58002
Copy link
Contributor

Acc. To you which approach is better

  1. Let the config options be defined on per argument bases and let the function each handle there config files on there own.
    Advantages :-
  • The option which can be placed in config and the arguments can be different and will be more flexible in the long run.
    Disadvantages :-
  • It requires more work (Somewhat) to implement any new function as then you have to consider that the config option will also need to be handled
  • Different structs need to be defined for the config and arguments and they need to be kept in sync as they can go out of sync if one is updated and other one is not.
  1. Let the options be Changed before they are passed to there respective function and change the values which come from the clap::Cli enum.
    Advantages :-
  • It is more always make sure that every command line option can also be set with a config file and two structs will not be needed to be kept in sync of each other.
    Disadvantages:-
  • It is very less ridget by design and will be hard to change if a function what to keep features only by either config or by arguments

I am not used to open source much. So if the thinks are say seem too basic or are wrong please do let me know.
Also there seem to be a config feature discussion going on in the clap crate which can solve this problem entirely.

clap-rs/clap#2763

@tyt2y3
Copy link
Member

tyt2y3 commented Dec 28, 2022

Sorry for the late reply, if you want to work on it, feel free and just do whatever you think makes the most sense!

I think we always want cli params to override the config file, so it's a two pass process, we first load a config file, and then override the options with the cli params. I think the serde struct can be shared with https://crates.io/crates/structopt thus removing code duplication.

@aadi58002
Copy link
Contributor

@tyt2y3 If you have suggestion for improvement let me know.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment