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

Fix #98 java server type includes /etc/default by default #140

Merged
merged 2 commits into from
Jan 30, 2014

Conversation

muuki88
Copy link
Contributor

@muuki88 muuki88 commented Jan 28, 2014

  • Added default configuration location
  • Added replacements in /etc/default configuration
  • Added tests
  • Added default configuration file with examples
  • Added /etc/default script to Upstart

* Added default configuration location
* Added replacements in /etc/default configuration
* Added tests
* Added default configuration file with examples
* Added /etc/default script to Upstart
@kardapoltsev
Copy link
Member

And how can I specify command line arguments to my program?

You put debian package properties inside package? I think /etc/default/ is basically for storing some parameters such as daemonUser.

+# Available replacements 
+# ------------------------------------------------
+# ${{author}}      debian author
+# ${{descr}}      debian package description

@muuki88
Copy link
Contributor Author

muuki88 commented Jan 29, 2014

I still have to dig a bit deeper in the startup script. There is an option for commandline parameters.

The replacements are just the default ones, available in every maintainerscript.

# Setting -X directly (-J is stripped)
# -J-X
# -J-Xmx 1024
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think this should be -J-Xmx1024 (no space).

@jsuereth
Copy link
Member

Nice work! I only have a minor comment.

@muuki88
Copy link
Contributor Author

muuki88 commented Jan 29, 2014

@aparkinson or @jsuereth if you're okay with this, then we merge this into master.

@jsuereth
Copy link
Member

Looks good to me!

aparkinson added a commit that referenced this pull request Jan 30, 2014
Fix #98 java server type includes /etc/default by default
@aparkinson aparkinson merged commit 832aed6 into master Jan 30, 2014
@kardapoltsev
Copy link
Member

And what about javaOptions in run? Could we pass this options to bash script?

@muuki88 muuki88 deleted the wip/etc-default branch January 30, 2014 09:03
@muuki88
Copy link
Contributor Author

muuki88 commented Jan 30, 2014

What I'm currently starting to think about is extending the claspath with a directory where you can place configuration files. As most of the typesafe products use Typesafe Config Library, config files could be easily picked up there.

IMHO we should keep configuration settings out of the start script. However there is a method to add app arguments, which is never called?

@aparkinson
Copy link
Contributor

@muuki88 Would having files in a directory effectively override existing config files packaged inside the jars?

Our Play! app has an internal safe config file and we use -Dconfig.file= to override the safe config with our production config

@muuki88
Copy link
Contributor Author

muuki88 commented Jan 30, 2014

Exactly. The standard behavior states all resources on classpath with this name, so you application should ship a reference.conf and you can place an application.conf in, e.g. /etc/your-app/.

The API for this must:

  • Add packageMapping for this directory
  • May add packageMapping for the config-file
  • prepend this folder to the classpath

What do you thinkg @aparkinson ?

@kardapoltsev
Copy link
Member

Sounds great for me.

@aparkinson
Copy link
Contributor

+1 I didn't know about that, sounds like the right way to go

@muuki88
Copy link
Contributor Author

muuki88 commented Jan 30, 2014

If will provide a PR for this.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants