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

Add an optional parameter --config for all sub commands #216

Closed
yottahmd opened this issue Jul 29, 2022 · 9 comments
Closed

Add an optional parameter --config for all sub commands #216

yottahmd opened this issue Jul 29, 2022 · 9 comments
Assignees
Milestone

Comments

@yottahmd
Copy link
Collaborator

yottahmd commented Jul 29, 2022

Why: this makes it easier to run multiple Dagu servers on a single instance.

@yottahmd yottahmd changed the title Add optional parameters --dags, --host and --port for dagu server command Add optional parameters --config for dagu server command Jul 29, 2022
@yottahmd yottahmd changed the title Add optional parameters --config for dagu server command Add an optional parameter --config for dagu server command Jul 29, 2022
@Arvintian
Copy link
Contributor

Should merge admin-configuration, env、admin config、global config into one config file, and the config file can specify by --config?

I think this way is clearer to config server or scheduler and easy to organize multiple instances's data.

@Arvintian
Copy link
Contributor

By the way, we can remove the rely of the $HOME path. Dagu's work dir can specify in the config file.

@Arvintian
Copy link
Contributor

Make dagu's execution environment don't rely on some sys environment. It is useful to run dagu on cloud or container platform. User can run dagu multi-instance or multi-tenancy.

https://github.com/yohamta/dagu/discussions/200

@yottahmd
Copy link
Collaborator Author

yottahmd commented Aug 1, 2022

Thanks for the great suggestions!

Should merge admin-configuration, env、admin config、global config into one config file, and the config file can specify by --config?

I think this way is clearer to config server or scheduler and easy to organize multiple instances's data.

I agree 100%. Good idea.

By the way, we can remove the rely of the $HOME path. Dagu's work dir can specify in the config file.

Yes, I was thinking of adding a config option home and DAGU__ADMIN_HOME env like $AIRFLOW_HOME in Airflow's config. What do you think?

@Arvintian
Copy link
Contributor

Arvintian commented Aug 1, 2022

I think config have a single entry is important. Currently admin's config some in admin.yaml, some in env. I am confused about it.
After review code, i found three type config.

  • For admin(server&scheduler) config.
  • For operator(start/stop...) and dag config.
  • For both admin and operator.

It's a little messy now :) and some code is coupling.

My idea:

  • env config is base, like $AIRFLOW_HOME, just about homedatabase basic config, for admin and operator but no business configuration.
  • admin config and global config should merge.
  • about dag config, it can understand not is config, it is dag object spec, should it is independent.

refer to kubernetes:

kubectl --kubeconfig ~/.kube/my-kube-config apply deployment-spec.yaml

dagu can as:

dagu --config my-dagu-config [start/stop/server] [dag-spec.yaml]

@yottahmd
Copy link
Collaborator Author

yottahmd commented Aug 1, 2022

Thanks for sharing the great idea!

I think config have a single entry is important. Currently admin's config some in admin.yaml, some in env. I am confused about it.

Yeah, let's make it to a single entry for config. I think it's best to have only env for basic config. Only home env is necessary for operator(start/stop/...) commands to find the admin config, for now, I think.

admin config and global config should merge.

How about having a new admin field baseConfig as below and eliminating the concept of global config? The baseConfig is for common settings shared among all DAGs such as env or mailOn etc.

baseConfig: <path/to/global-config-yaml>

For the other idea, I agree 100% :)

@Arvintian
Copy link
Contributor

Follow this pattern:

dagu --config my-dagu-config [start/stop/server] [dag-spec.yaml]

I think env is for dagu's basic work, link save database、logs、suspend eg. Some default configs value can use default env value to set default value, link data path eg. But it is not use to find config, it is use to dagu self work, like save state、cache eg.

Only home env is necessary for operator(start/stop/...) commands to find the admin config, for now, I think.

--config can tell all components where to find config file. For now default value, can point to default value ($home[~/.dagu]/admin.yaml), this behavior is compatible fallback.

baseConfig is a good idea for common settings among all DAGs, it's default value can be ($home[~/.dagu]/config.yaml) for compatible fallback, i agree about it 👍 .

@yottahmd
Copy link
Collaborator Author

yottahmd commented Aug 2, 2022

Fantastic! I agree completely 👍 .

@yottahmd yottahmd changed the title Add an optional parameter --config for dagu server command Add an optional parameter --config for all sub commands Aug 10, 2022
@yottahmd yottahmd added this to the v1.7.0 milestone Aug 11, 2022
@yottahmd yottahmd self-assigned this Aug 11, 2022
@yottahmd
Copy link
Collaborator Author

done!

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

2 participants