new:dev: Upgrade thrift & maven-thrift-plugin. Make tests relocatable #46
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
added 2 commits
July 17, 2017 13:59
Upgraded thrift from 0.9.0 to 0.9.3. The main motivation is that 0.9.3 can be installed through brew on mac. I tried to upgrade to thrift 0.10.0 but there were test failures. Another improvement is that tests assumed specific directory structures internal to Qubole. The test directories are now configured relative to `java.io.tmpdir`.
| @@ -106,7 +106,8 @@ public void run() | |||
| log.info("Stopping Local Transfer server"); | |||
| } | |||
| catch (IOException e) { | |||
| log.error(String.format("Error starting Local Transfer server %s", e)); | |||
| log.error(String.format("Error starting Local Transfer server %s", e), e); | |||
| e.printStackTrace(); | |||
There was a problem hiding this comment.
wasnt log logging exceptions? We are using the format where exception should be logged
There was a problem hiding this comment.
It does but just the message. I wanted stack trace but this didnt work out as well. I'll revert this change.
| import org.testng.annotations.AfterMethod; | ||
| import org.testng.annotations.BeforeMethod; | ||
| import org.testng.annotations.Test; | ||
| import org.testng.annotations.*; |
There was a problem hiding this comment.
Didnt checkstyle crib about this compaction?
Easier to read import without compacting them to '*', we should revert this.
There was a problem hiding this comment.
No. it didnt. Test files are ignored ?
There was a problem hiding this comment.
Ya, looks like it. Tried and it fails for main code and passes for tests.
|
lgtm, couple of minor comments. |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Upgraded thrift from 0.9.0 to 0.9.3. The main motivation is that
0.9.3 can be installed through brew on mac. I tried to upgrade to
thrift 0.10.0 but there were test failures.
Upgrade maven-thrift-plugin to 0.1.11. 0.1.10 is not available anymore
in http://maven.davidtrott.com/repository. 0.1.11 is available in apache
maven repository.
Another improvement is that tests assumed specific directory
structures internal to Qubole. The test directories are now
configured relative to
java.io.tmpdir.