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 commons-daemon from dependencies #348
Conversation
Motivation: The current version of `jsvc` does not support Java 9+. If we can remove `commons-daemon` from dependencies, we would have less blocker for supporting latest Java version. Modifications: - Removed `commons-daemon` from dependencies. - Removed `jsvc` binaries from `dist/src/bin`. - The server now creates a pidfile when it starts up and deletes it when it is terminated. - `startup`/`shutdown` waits until the pidfile is created/deleted. - Updated `Dockerfile`: - Using `openjdk:lastest` instead of `openjdk:8`. - Set the entry point in order to correctly handle a signal from a user. `docker stop` command, which sends `TERM` signal to the container, will work. Result: - Less dependencies. - Support Java 9+. - Closes line#277
ddaeae3
to
cd16218
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
How about using jnr-posix
?
pid = POSIXFactory.getPOSIX().getpid();
server/src/main/java/com/linecorp/centraldogma/server/Main.java
Outdated
Show resolved
Hide resolved
Codecov Report
@@ Coverage Diff @@
## master #348 +/- ##
============================================
- Coverage 66% 65.85% -0.16%
+ Complexity 2644 2642 -2
============================================
Files 306 306
Lines 11047 11063 +16
Branches 1211 1214 +3
============================================
- Hits 7292 7285 -7
- Misses 3055 3077 +22
- Partials 700 701 +1
Continue to review full report at Codecov.
|
Codecov Report
@@ Coverage Diff @@
## master #348 +/- ##
===========================================
- Coverage 66% 65.9% -0.11%
- Complexity 2644 2646 +2
===========================================
Files 306 306
Lines 11047 11062 +15
Branches 1211 1214 +3
===========================================
- Hits 7292 7290 -2
- Misses 3055 3074 +19
+ Partials 700 698 -2
Continue to review full report at Codecov.
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Just two nits. Thanks!
server/src/main/java/com/linecorp/centraldogma/server/Main.java
Outdated
Show resolved
Hide resolved
server/src/main/java/com/linecorp/centraldogma/server/Main.java
Outdated
Show resolved
Hide resolved
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Good job!
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
👍
Motivation: A user could configure the title of web-based administrative console by line#348. But it is only available to add some text to the end of pre-defined title which is `Central Dogma at ..hostname..`. We think it would be more useful if a user can customize the entire title of the web console. Modifications: - Made the entire title of the web console customizable. - A user can use ``{{hostname}}`` as a variable of the actual hostname that the server is running on. - Updated site document. - Misc - Try to move pidfile again silently when it caught `AtomicMoveNotSupportedException`. Result: - Closes line#43 - Usability.
Motivation: A user could configure the title of web-based administrative console by #348. But it is only available to add some text to the end of pre-defined title which is `Central Dogma at ..hostname..`. We think it would be more useful if a user can customize the entire title of the web console. Modifications: - Made the entire title of the web console customizable. - A user can use ``{{hostname}}`` as a variable of the actual hostname that the server is running on. - Updated site document. - Misc - Try to move pidfile again silently when it caught `AtomicMoveNotSupportedException`. Result: - Closes #43 - Usability.
Motivation:
The current version of
jsvc
does not support Java 9+.If we can remove
commons-daemon
from dependencies, we would have less blocker for supporting latest Java version.Modifications:
commons-daemon
from dependencies.jsvc
binaries fromdist/src/bin
.startup
/shutdown
waits until the pidfile is created/deleted.Dockerfile
:openjdk:lastest
instead ofopenjdk:8
.docker stop
command, which sendsTERM
signal to the container, will work.Result: