You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@drill.apache.org by sohami <gi...@git.apache.org> on 2018/04/05 22:50:23 UTC
[GitHub] drill pull request #1202: DRILL-6311: No logging information in drillbit.log...
GitHub user sohami opened a pull request:
https://github.com/apache/drill/pull/1202
DRILL-6311: No logging information in drillbit.log / drillbit.out
You can merge this pull request into a Git repository by running:
$ git pull https://github.com/sohami/drill DRILL-6311
Alternatively you can review and apply these changes as the patch at:
https://github.com/apache/drill/pull/1202.patch
To close this pull request, make a commit to your master/trunk branch
with (at least) the following in the commit message:
This closes #1202
----
commit c988dc83df36da6dbe4d9ab77595084e9cd084f1
Author: Sorabh Hamirwasia <sh...@...>
Date: 2018-04-05T22:49:22Z
DRILL-6311: No logging information in drillbit.log / drillbit.out
----
---
[GitHub] drill pull request #1202: DRILL-6311: No logging information in drillbit.log...
Posted by vrozov <gi...@git.apache.org>.
Github user vrozov commented on a diff in the pull request:
https://github.com/apache/drill/pull/1202#discussion_r180275891
--- Diff: exec/vector/pom.xml ---
@@ -69,6 +69,7 @@
<groupId>org.apache.drill</groupId>
<artifactId>drill-common</artifactId>
<version>${project.version}</version>
+ <scope>test</scope>
--- End diff --
OK, I see. @sohami I'd suggest moving the dependency to the parent exec module pom.
---
[GitHub] drill pull request #1202: DRILL-6311: No logging information in drillbit.log...
Posted by vrozov <gi...@git.apache.org>.
Github user vrozov commented on a diff in the pull request:
https://github.com/apache/drill/pull/1202#discussion_r180249547
--- Diff: exec/vector/pom.xml ---
@@ -69,6 +69,7 @@
<groupId>org.apache.drill</groupId>
<artifactId>drill-common</artifactId>
<version>${project.version}</version>
+ <scope>test</scope>
--- End diff --
I mean that there are no unit tests that are annotated with `@Category(SlowTest.class)` or any other `@Category`, so it is not clear why the dependency is necessary.
---
[GitHub] drill issue #1202: DRILL-6311: No logging information in drillbit.log / dril...
Posted by vrozov <gi...@git.apache.org>.
Github user vrozov commented on the issue:
https://github.com/apache/drill/pull/1202
LGTM +1.
---
[GitHub] drill pull request #1202: DRILL-6311: No logging information in drillbit.log...
Posted by vrozov <gi...@git.apache.org>.
Github user vrozov commented on a diff in the pull request:
https://github.com/apache/drill/pull/1202#discussion_r179626383
--- Diff: exec/vector/pom.xml ---
@@ -69,6 +69,7 @@
<groupId>org.apache.drill</groupId>
<artifactId>drill-common</artifactId>
<version>${project.version}</version>
+ <scope>test</scope>
--- End diff --
@ilooner why was the dependency introduced?
---
[GitHub] drill pull request #1202: DRILL-6311: No logging information in drillbit.log...
Posted by ilooner <gi...@git.apache.org>.
Github user ilooner commented on a diff in the pull request:
https://github.com/apache/drill/pull/1202#discussion_r180247017
--- Diff: exec/vector/pom.xml ---
@@ -69,6 +69,7 @@
<groupId>org.apache.drill</groupId>
<artifactId>drill-common</artifactId>
<version>${project.version}</version>
+ <scope>test</scope>
--- End diff --
The `<classifier>tests</classifier>` tag includes the test classes in drill-common. The test category classes are included in common/src/test/java/org/apache/drill/categories. So test category classes will be accessible to maven. However, looks like I forgot the test scope so the test classes and resources from drill-common were being included in Drill's standard (non-test) classpath.
---
[GitHub] drill pull request #1202: DRILL-6311: No logging information in drillbit.log...
Posted by asfgit <gi...@git.apache.org>.
Github user asfgit closed the pull request at:
https://github.com/apache/drill/pull/1202
---
[GitHub] drill pull request #1202: DRILL-6311: No logging information in drillbit.log...
Posted by ilooner <gi...@git.apache.org>.
Github user ilooner commented on a diff in the pull request:
https://github.com/apache/drill/pull/1202#discussion_r180252832
--- Diff: exec/vector/pom.xml ---
@@ -69,6 +69,7 @@
<groupId>org.apache.drill</groupId>
<artifactId>drill-common</artifactId>
<version>${project.version}</version>
+ <scope>test</scope>
--- End diff --
@vrozov As we discussed offline drill-common-tests is not a dependency of the vector project itself, it is a dependency required by a maven invocation when a category is specified. For example in `mvn test -DexcludedGroups="org.apache.drill.categories.SlowTest,org.apache.drill.categories.UnlikelyTest,org.apache.drill.categories.SecurityTest"` the category classes are required by maven. However when `mvn test` is used the category classes are not required by maven.
---
[GitHub] drill pull request #1202: DRILL-6311: No logging information in drillbit.log...
Posted by ilooner <gi...@git.apache.org>.
Github user ilooner commented on a diff in the pull request:
https://github.com/apache/drill/pull/1202#discussion_r180205787
--- Diff: exec/vector/pom.xml ---
@@ -69,6 +69,7 @@
<groupId>org.apache.drill</groupId>
<artifactId>drill-common</artifactId>
<version>${project.version}</version>
+ <scope>test</scope>
--- End diff --
@vrozov JUnit supports including or excluding tests based on categories. Categories are just marker interfaces that are passed to the `@Category` annotation. For example a test class annotated with `@Category(SlowTest.Class)` will be excluded from a Travis run since Travis excludes the SlowTest category. All the test categories needed to be included in a common artifact since all the drill submodules require the test categories. So the test categories were added to drill-common.
If drill-common is not included mvn would fail with an error when running tests marked with `@Category(SlowTest.class)` since JUnit would not be able to find the SlowTest class.
---
[GitHub] drill issue #1202: DRILL-6311: No logging information in drillbit.log / dril...
Posted by sohami <gi...@git.apache.org>.
Github user sohami commented on the issue:
https://github.com/apache/drill/pull/1202
@vrozov - Update the PR based on your suggestion. The command `mvn test -DexcludedGroups="org.apache.drill.categories.SlowTest,org.apache.drill.categories.UnlikelyTest,org.apache.drill.categories.SecurityTest"` was not working on latest master as well. Spoke with @ilooner and made sure atleast the command used in Travis is working with this change `mvn install -DexcludedGroups="org.apache.drill.categories.SlowTest,org.apache.drill.categories.UnlikelyTest,org.apache.drill.categories.SecurityTest"`
---
[GitHub] drill pull request #1202: DRILL-6311: No logging information in drillbit.log...
Posted by vrozov <gi...@git.apache.org>.
Github user vrozov commented on a diff in the pull request:
https://github.com/apache/drill/pull/1202#discussion_r180226298
--- Diff: exec/vector/pom.xml ---
@@ -69,6 +69,7 @@
<groupId>org.apache.drill</groupId>
<artifactId>drill-common</artifactId>
<version>${project.version}</version>
+ <scope>test</scope>
--- End diff --
How it worked before? In the commit that introduced the test dependency, there is no test categorization.
---
[GitHub] drill issue #1202: DRILL-6311: No logging information in drillbit.log / dril...
Posted by sohami <gi...@git.apache.org>.
Github user sohami commented on the issue:
https://github.com/apache/drill/pull/1202
@vrozov - Please help to review.
---