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

Issue-51: Prints Process Logs in RuntimeException Message #10

Merged
merged 1 commit into from
Mar 12, 2020

Conversation

ashok2ashok
Copy link

@ashok2ashok ashok2ashok commented Mar 12, 2020

This resolves the issue: kstyrc#51

While resolving logs at the process level is preferred as specified in the pull-request: kstyrc#113 - this fix takes a much more conservative approach and prints the redis process logs only in case of an error/exception when the server startup pattern is not found. That way we do not need to worry about logs in case of successful redis server starts.

Removed @override as they are not needed when implementing interfaces.

Before this fix:

java.lang.RuntimeException: Can't start redis server. Check logs for details.
	at redis.embedded.AbstractRedisInstance.awaitRedisServerReady(AbstractRedisInstance.java:67)
	at redis.embedded.AbstractRedisInstance.start(AbstractRedisInstance.java:29)
	at redis.embedded.RedisServer.start(RedisServer.java:9)

After this fix error log:

java.lang.RuntimeException: Can't start redis server. Check logs for details. Redis process log: 
[63038] 12 Mar 14:16:06.220 # Creating Server TCP listening socket 127.0.0.1:6379: bind: Address already in use
	at redis.embedded.AbstractRedisInstance.awaitRedisServerReady(AbstractRedisInstance.java:60)
	at redis.embedded.AbstractRedisInstance.start(AbstractRedisInstance.java:36)
	at redis.embedded.RedisServer.start(RedisServer.java:9)

Without the above fix, the workaround the devs need to write is not perfect like below. The exception message e.getMessage() can't be checked for exact cause. We can only guess at it:

//    Keep for embedded Redis config testing
    public static void main(String... args) throws Exception {
        int port = 6379;
        RedisServer redisServer = null;
        try{
            redisServer = new RedisServer(port);
            redisServer.start();
        } catch(RuntimeException e) {
            if (e.getMessage().contains("Can't start redis server")) {
                int freePort = SocketUtils.findAvailableTcpPort();
                LOGGER.warn("{} Port already in use. Changing port to {}", port, freePort);
                redisServer = new RedisServer(freePort);
                redisServer.start();
            }
        }
    }

@ashok2ashok
Copy link
Author

ashok2ashok commented Mar 12, 2020

@robertotru - Looks like Java 8 installation in Travis is failing. It's expecting a feature between 9 to 15, but the script has 8. Please correct the build configs:

travis_fold:start:install_jdk
�[0K�[33;1mInstalling oraclejdk8�[0m
$ export JAVA_HOME=~/oraclejdk8
$ export PATH="$JAVA_HOME/bin:$PATH"
$ ~/bin/install-jdk.sh --target "/home/travis/oraclejdk8" --workspace "/home/travis/.cache/install-jdk" --feature "8" --license "BCL"
Ignoring license option: BCL -- using GPLv2+CE by default
install-jdk.sh 2020-01-14
Expected feature release number in range of 9 to 15, but got: 8
�[31;1mThe command "~/bin/install-jdk.sh --target "/home/travis/oraclejdk8" --workspace "/home/travis/.cache/install-jdk" --feature "8" --license "BCL"" failed and exited with 3 during .�[0m

Your build has been stopped.

Ref:

@ashok2ashok ashok2ashok force-pushed the master branch 2 times, most recently from 9ba2040 to 47745b0 Compare March 12, 2020 20:36
This resolves the issue: kstyrc#51

While resolving logs at the process level is preferred as specified in the pull-request: kstyrc#113 - this fix takes a much more conservative approach and prints the redis process logs only in case of an error/exception when the server startup pattern is not found. That way we do not need to worry about logs in case of successful redis server starts.

Dist updated to trusty to resolves travis jdk8 build issue caused by xenial dist

Removed sudo and curl commands as they are not longer supported.
jpm4j is not longer functional and sudo is deprecated in travis
@ashok2ashok
Copy link
Author

@robertotru - I fixed the .travis.yml build script to:

  • Use trusty distribution as the default xenial has issues with oraclejdk8
  • Removed the jpm4j package manager as it's no longer functional
  • Removed sudo: false as it's deprecated and no longer needed.

The build is successful now and is ready to be merged.

@robertotru robertotru merged commit aeef6be into ozimov:master Mar 12, 2020
@ashok2ashok ashok2ashok changed the title Issue-51: Prints Process Logs in RuntimeException Message Issue-51: Prints Process Logs in RuntimeException Message Mar 12, 2020
@ashok2ashok ashok2ashok changed the title Issue-51: Prints Process Logs in RuntimeException Message Issue-51: Prints Process Logs in RuntimeException Message Mar 12, 2020
@lploom
Copy link

lploom commented Apr 9, 2020

Can we get a release please?

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.

4 participants