You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@drill.apache.org by GitBox <gi...@apache.org> on 2021/11/14 19:14:36 UTC
[GitHub] [drill] pjfanning opened a new pull request #2375: Update contrib/storage-hive copy of log4j Strings.java
pjfanning opened a new pull request #2375:
URL: https://github.com/apache/drill/pull/2375
# [DRILL-2373](https://issues.apache.org/jira/browse/DRILL-2373): Update contrib/storage-hive copy of log4j Strings.java
Relates to pjfanning/excel-streaming-reader#76. This is an alternate to https://github.com/apache/drill/pull/2374.
## Description
(https://github.com/apache/drill/blob/master/contrib/storage-hive/core/src/main/java/org/apache/logging/log4j/util/Strings.java is a copy of the real Log4j class but causes issues for Apache POI Excel code that relies on the latest Log4j code.)
## Documentation
Shouldn't require doc change
## Testing
unit tests
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: dev-unsubscribe@drill.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [drill] cgivre commented on pull request #2375: DRILL-8044: Update contrib/storage-hive copy of log4j Strings.java
Posted by GitBox <gi...@apache.org>.
cgivre commented on pull request #2375:
URL: https://github.com/apache/drill/pull/2375#issuecomment-968955479
@vvysotskyi Will we know if this prevents Hive queries from functioning?
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: dev-unsubscribe@drill.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [drill] vvysotskyi commented on pull request #2375: Update contrib/storage-hive copy of log4j Strings.java
Posted by GitBox <gi...@apache.org>.
vvysotskyi commented on pull request #2375:
URL: https://github.com/apache/drill/pull/2375#issuecomment-968579742
I'll restart Travis CI tests now
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: dev-unsubscribe@drill.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [drill] pjfanning commented on pull request #2375: Update contrib/storage-hive copy of log4j Strings.java
Posted by GitBox <gi...@apache.org>.
pjfanning commented on pull request #2375:
URL: https://github.com/apache/drill/pull/2375#issuecomment-968373982
@vvysotskyi one of the travis-ci runs failed due a forked jvm running tests crashing - this happened before the module with the change was reached - is there anyway to get the tests rerun?
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: dev-unsubscribe@drill.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [drill] cgivre commented on pull request #2375: DRILL-8044: Update contrib/storage-hive copy of log4j Strings.java
Posted by GitBox <gi...@apache.org>.
cgivre commented on pull request #2375:
URL: https://github.com/apache/drill/pull/2375#issuecomment-971581391
@pjfanning @vvysotskyi
Thank you for all your work on this. I restarted the CI, and assuming it passes this time, I'll merge later today.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: dev-unsubscribe@drill.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [drill] cgivre commented on pull request #2375: DRILL-8044: Update contrib/storage-hive copy of log4j Strings.java
Posted by GitBox <gi...@apache.org>.
cgivre commented on pull request #2375:
URL: https://github.com/apache/drill/pull/2375#issuecomment-970676729
> @vvysotskyi made the log4j dependency change but the CI/CD build timed out when trying to get the pom - seems like an env issue at time of build
There's a major google issue happening right now. I'll restart the CI once everything finishes. It looks like everything is passing.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: dev-unsubscribe@drill.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [drill] pjfanning commented on pull request #2375: DRILL-8044: Update contrib/storage-hive copy of log4j Strings.java
Posted by GitBox <gi...@apache.org>.
pjfanning commented on pull request #2375:
URL: https://github.com/apache/drill/pull/2375#issuecomment-968952893
@cgivre You reported the initial issue - would you be able to try this out with a POI 5.1.0 (and excel-streaming-reader 3.2.3)?
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: dev-unsubscribe@drill.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [drill] vvysotskyi commented on pull request #2375: Update contrib/storage-hive copy of log4j Strings.java
Posted by GitBox <gi...@apache.org>.
vvysotskyi commented on pull request #2375:
URL: https://github.com/apache/drill/pull/2375#issuecomment-968579503
@pjfanning, since this change is required for updating poi and excel-streaming-reader, could you please update them here and ensure that the issue is resolved?
(Version update was reverted in this commit: https://github.com/apache/drill/commit/56ff6556b83ca0c3364592ade73d9e7436fcab44)
Also, please include the Apache Jira ticket into the commit message and update references in this PR.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: dev-unsubscribe@drill.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [drill] cgivre merged pull request #2375: DRILL-8044: Update contrib/storage-hive copy of log4j Strings.java
Posted by GitBox <gi...@apache.org>.
cgivre merged pull request #2375:
URL: https://github.com/apache/drill/pull/2375
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: dev-unsubscribe@drill.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [drill] pjfanning commented on pull request #2375: DRILL-8044: Update contrib/storage-hive copy of log4j Strings.java
Posted by GitBox <gi...@apache.org>.
pjfanning commented on pull request #2375:
URL: https://github.com/apache/drill/pull/2375#issuecomment-970665696
@vvysotskyi made the log4j dependency change but the CI/CD build timed out when trying to get the pom - seems like an env issue at time of build
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: dev-unsubscribe@drill.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [drill] vvysotskyi commented on pull request #2375: DRILL-8044: Update contrib/storage-hive copy of log4j Strings.java
Posted by GitBox <gi...@apache.org>.
vvysotskyi commented on pull request #2375:
URL: https://github.com/apache/drill/pull/2375#issuecomment-968974506
@cgivre, it means that excell tests should be improved to use that code path, also there might be some additional dependencies for a test scope, so tests path, but actually it didn't work.
No need to check Hive, it wouldn't be broken with these changes
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: dev-unsubscribe@drill.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [drill] cgivre commented on pull request #2375: DRILL-8044: Update contrib/storage-hive copy of log4j Strings.java
Posted by GitBox <gi...@apache.org>.
cgivre commented on pull request #2375:
URL: https://github.com/apache/drill/pull/2375#issuecomment-968954849
@pjfanning
Sure thing! I'll build and report back this evening. (I'm on ET in the US)
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: dev-unsubscribe@drill.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [drill] cgivre commented on pull request #2375: DRILL-8044: Update contrib/storage-hive copy of log4j Strings.java
Posted by GitBox <gi...@apache.org>.
cgivre commented on pull request #2375:
URL: https://github.com/apache/drill/pull/2375#issuecomment-968965347
@vvysotskyi
The reason I'm asking is that the Excel unit tests all worked for the Excel plugin, but it wasn't until I manually ran a query, things broke. I don't have a Hive installation to manually test.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: dev-unsubscribe@drill.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [drill] vvysotskyi commented on pull request #2375: DRILL-8044: Update contrib/storage-hive copy of log4j Strings.java
Posted by GitBox <gi...@apache.org>.
vvysotskyi commented on pull request #2375:
URL: https://github.com/apache/drill/pull/2375#issuecomment-968961029
@cgivre, hive unit tests would catch such issues, changes in this PR doesn't look to break the Hive
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: dev-unsubscribe@drill.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [drill] vvysotskyi commented on pull request #2375: DRILL-8044: Update contrib/storage-hive copy of log4j Strings.java
Posted by GitBox <gi...@apache.org>.
vvysotskyi commented on pull request #2375:
URL: https://github.com/apache/drill/pull/2375#issuecomment-970578028
@pjfanning, looks like POI has a hard dependency on log4j. Could you please add a dependency on `log4j-to-slf4j` to `pom.xml` to allow using that library?
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: dev-unsubscribe@drill.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org