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

Remove cmd package #2526

Open
asim opened this issue Jul 11, 2022 · 8 comments
Open

Remove cmd package #2526

asim opened this issue Jul 11, 2022 · 8 comments

Comments

@asim
Copy link
Member

asim commented Jul 11, 2022

Looking at removing the cmd package. I think flag and env var parsing was initially quite useful when the surface area of go-micro was small. Over time that's gotten broader and more complex to manage. Ultimately if go-micro is pluggable, then either the cmd package needs to be totally rewritten or removed entirely.

@asim
Copy link
Member Author

asim commented Jul 11, 2022

The protobuf code generator has moved to https://github.com/go-micro/generator

@Davincible
Copy link
Member

Davincible commented Jul 11, 2022

Could you elaborate a bit more? Personally I still configure my go-micro services completely with env vars as its most versatile and it's easier to change a single .env file compared to cmd args in a k8s deployment.

What would need to be rewritten and why?

@asim
Copy link
Member Author

asim commented Jul 11, 2022

The cmd package was mostly a hack. It's not a pluggable one. It's not something where you can pick and choose your parsers. It would make more sense as a config package with configurable functions to execute the parsing. Right now it's a very hard coded fixed thing. I think it would make more sense as a separate utility or to just be in the util dir.

@vtolstov
Copy link
Contributor

If you prefer to specify config stuff in config struct tags you can use something like this (not fully compatible with go-micro)
https://github.com/unistack-org/micro-config-flag/blob/v3/flag_test.go#L27

@asim
Copy link
Member Author

asim commented Jul 11, 2022

I think it makes sense. And to maybe even rewrite the config package to enable very easy parsing and access to stuff like this for env and flags to do the same as what was occurring before but be even more flexible so that the user can essentially parse all the things they need. In micro we rewrote the config interface for that.

https://github.com/micro/micro/blob/master/service/config/config.go#L26

@Davincible
Copy link
Member

Davincible commented Sep 24, 2022

@asim just to double check, you were talking about the package that is now at util/cmd/cmd.go ?

@asim
Copy link
Member Author

asim commented Sep 24, 2022

Yea, it's convenient for initialisation but not a mandatory feature

@keepstep
Copy link
Contributor

keepstep commented Oct 7, 2022

it is a problem that cmd's default option will overwrite options that code in srv.Init,
so i must call srv.Init again if want option effect

// Create service  
srv := micro.NewService()  
// cmd will overwrite some option like "micro.RegisterInterval micro.RegisterTTL"  
srv.Init(  
    micro.RegisterInterval(time.Second * 5), 
    micro.RegisterTTL(time.Second * 10),  
)
// must init again   
srv.Init(  
   micro.RegisterInterval(time.Second * 5),  
   micro.RegisterTTL(time.Second * 10),  
)  

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

4 participants