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

[partial][common] runner #64

Merged
merged 11 commits into from Jan 3, 2017

Conversation

Projects
None yet
1 participant
@at15
Copy link
Member

at15 commented Dec 20, 2016

Related #63, #65

This should not only make runner more general but also solve some old problems like #56 #58 .
Currently the focus is on

  • more expressive config file
  • allow running process in background, act like a process manager, (Ayi itself should also be able to run like a daemon, may provide some api for short life Ayi instance to communicate with it?)
  • abstract the config parsing, make viper one of the adapter, not bound with it

@at15 at15 added this to the 0.1.0 milestone Dec 20, 2016

@at15 at15 self-assigned this Dec 20, 2016

at15 added some commits Dec 20, 2016

[common][runner] Move Command to runner package
- It seems #58 is not caused by shell quote
- Rename `Command` to `NewCmd`, the first one looks like a constructor
[common][runner] Add RunCommand
- Add notes about #58 in test, shell expand wildchar while `os/exec`
  simply pass it as it is.
[cmd] Dynamic register cmd won't work
- only work when put it in `init`, put `AddCommand` in `initConfig`
  does not work
- TODO: may edit RootCmd's Execute?
[cmd] Can register command in RootCmd.Execute
- the main function call Execute to actually run cobra, and at that
  time the configuration file is properly loaded, command line arguments are parsed
  but you can't access it from cobra I guess?
- NOTE: initConfig is actually a global pre run for each command, so it
  works when run `Ayi help`, but won't work for `Ayi dummy`
[cmd] Remove Dynamic Commands
- flags are only parsed when executing the command,
  so the Execute wrapper can only use fixed configuration file
- the yaml syntax for the runner may become very verbose
[common][runner] Add NewCmdWithAutoShell
- use shell instead of `os/exec` when common exectuable like `rm` is detected
  because normally they need shell expansion for their arguments
- TODO: change the array to set (syntax sugar for map[string]bool
[common][structure] Init Set structure
- non thread safe, only support string key, and only have Contains
  method
- replace usage in command
- replace usage in log PkgFilter

@at15 at15 modified the milestones: 0.0.4, 0.1.0 Dec 22, 2016

at15 added some commits Dec 23, 2016

[common][runner][spec] Add new config example
- support define command in object
- support run command in background
- support run in parallel

@at15 at15 referenced this pull request Dec 26, 2016

Merged

[partial] TSDB Proxy Client #19

6 of 22 tasks complete
@at15

This comment has been minimized.

Copy link
Member Author

at15 commented Jan 2, 2017

currently #63 has too much, may need to split the pull request into several small one, and maybe it's time to get ride of viper, first logrus then viper, guess there isn't much time left for cobra.

@at15 at15 changed the title [WIP][common] runner [partial][common] runner Jan 3, 2017

@at15

This comment has been minimized.

Copy link
Member Author

at15 commented Jan 3, 2017

LGTM

Approved with PullApprove

@at15 at15 merged commit 691a4d6 into master Jan 3, 2017

4 checks passed

code-review/pullapprove Approved by at15
Details
continuous-integration/travis-ci/pr The Travis CI build passed
Details
continuous-integration/travis-ci/push The Travis CI build passed
Details
coverage/coveralls First build on common-util/config at 58.094%
Details

@at15 at15 deleted the common-util/runner branch Apr 4, 2018

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