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

Making it possible to configure Moco through Groovy script #45

Closed
wants to merge 36 commits into from

Conversation

marcingrzejszczak
Copy link
Contributor

Together with @kurlenda we created a bash script that allows to configure and run Moco server through Groovy script. In addition it downloads groovy if one doesn't have GDK already installed on his drive.

@dreamhead
Copy link
Owner

Some comments for the commit.

  • Help message is not consistent with filename: one is run.sh, the other is gmoco.sh
  • It would be better if the argument is consistent with original standalone, moco shell could be a reference.
  • I didn't fully understand why there is moco_config_script.groovy.

@marcingrzejszczak
Copy link
Contributor Author

  1. It's a typo - of course it should be gmoco.sh
  2. I'll check out the moco shell and try to adapt the script
  3. The idea is such that imagine that you have several different teams that would like to benefit from moco executed from groovy scripts. Each of the teams would have its own configuration script that they would like to run. Let's say that you have a Foo team and a Bar team. Each of the teams have 20 different services that they would like to mock. So one team has foo_config_script.groovy and the other bar_config_script.groovy. In this way team Foo runs the script ./gmoco.sh 9090 foo_config_script and team Bar runs ./gmoco.sh 9595 bar_config_script. The moco_config_script is an example of how to write such a configuration - that the script returns an array of closures that are iterated over and executed in the moco_script.groovy

… Alttered moco_script.groovy to call full groovy config script instead of appending .groovy to the script name
@marcingrzejszczak
Copy link
Contributor Author

  1. Fixed typo
  2. Tried to adapt script to moco
  3. Added additional checks on passed arguments

@dreamhead
Copy link
Owner

Moco shell will download Moco from Maven repository if there is no Moco installed locally. On the other hand, it can upgrade Moco to latest version.

It would be better if gmoco shell supports the same behaviour.

…here is no Moco installed locally. It downloads the latest version possible.
@marcingrzejszczak
Copy link
Contributor Author

The script works exactly as requested

@dreamhead
Copy link
Owner

I've merged this pull request. Some improvement suggestions are as follow:

  • The behaviour of gmoco is different from moco, because it will run as a daemon. There are two ways to make is easy-use in my mind: run it as normal process like moco, or tell user in where the log can be found for debugging.
  • I'm not sure if moco_pid is a good name, maybe .moco_pid and if it possible to delete it after shutting down gmoco.
  • The generated file can be ignored in .gitignore file.

@marcingrzejszczak
Copy link
Contributor Author

That's great! I'll try to take a look at it in free time.

2. Refactored names of scripts
3. Added generated log and pid files to .gitgnore
4. Added shutdown hook for deamon version of moco that deletes the pid file
5. Refactored code
@marcingrzejszczak
Copy link
Contributor Author

Commited all of the suggestions for improvements:

  1. Added switch (-d) that allows user to run moco in deamon mode or not
  2. Refactored names of scripts
  3. Added generated log and pid files to .gitgnore
  4. Added shutdown hook for deamon version of moco that deletes the pid file
  5. Refactored code

@marcingrzejszczak
Copy link
Contributor Author

Any chances of pushing the changes and closing the PR? ;)

dreamhead added a commit that referenced this pull request Feb 1, 2014
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

Successfully merging this pull request may close these issues.

3 participants