You are viewing a plain text version of this content. The canonical link for it is here.
Posted to reviews@spark.apache.org by GitBox <gi...@apache.org> on 2020/07/10 19:17:37 UTC
[GitHub] [spark] frankyin-factual opened a new pull request #29069: [SPARK-31831] Use constructor for mock in HiveSessionImplSuite
frankyin-factual opened a new pull request #29069:
URL: https://github.com/apache/spark/pull/29069
<!--
Thanks for sending a pull request! Here are some tips for you:
1. If this is your first time, please read our contributor guidelines: https://spark.apache.org/contributing.html
2. Ensure you have added or run the appropriate tests for your PR: https://spark.apache.org/developer-tools.html
3. If the PR is unfinished, add '[WIP]' in your PR title, e.g., '[WIP][SPARK-XXXX] Your PR title ...'.
4. Be sure to keep the PR description updated to reflect all changes.
5. Please write your PR title to summarize what this PR proposes.
6. If possible, provide a concise example to reproduce the issue for a faster review.
7. If you want to add a new configuration, please read the guideline first for naming configurations in
'core/src/main/scala/org/apache/spark/internal/config/ConfigEntry.scala'.
-->
### What changes were proposed in this pull request?
Fix flaky test org.apache.spark.sql.hive.thriftserver.HiveSessionImplSuite by using mockito constructor instantiator to avoid classloader issue.
### Why are the changes needed?
It causes build instability.
### Does this PR introduce _any_ user-facing change?
No.
### How was this patch tested?
It is a fix for a flaky test, but need to run multiple times against Jenkins.
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] AmplabJenkins commented on pull request #29069: [SPARK-31831][SQL][TESTS] Use constructor for mock in HiveSessionImplSuite
Posted by GitBox <gi...@apache.org>.
AmplabJenkins commented on pull request #29069:
URL: https://github.com/apache/spark/pull/29069#issuecomment-657081269
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] SparkQA removed a comment on pull request #29069: [SPARK-31831][SQL][TESTS] Use subclasses for mock in HiveSessionImplSuite
Posted by GitBox <gi...@apache.org>.
SparkQA removed a comment on pull request #29069:
URL: https://github.com/apache/spark/pull/29069#issuecomment-657159524
**[Test build #125701 has started](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/125701/testReport)** for PR 29069 at commit [`6771504`](https://github.com/apache/spark/commit/6771504eb87c3cf8a6f23f1657c93509c3932041).
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] HeartSaVioR commented on pull request #29069: [SPARK-31831][SQL][TESTS] Use constructor for mock in HiveSessionImplSuite
Posted by GitBox <gi...@apache.org>.
HeartSaVioR commented on pull request #29069:
URL: https://github.com/apache/spark/pull/29069#issuecomment-656953089
Unfortunately it turned out my patch didn't fix the issue (so I'm OK to revert my patch if we want to) - that said, I'd like to see the analysis how it fixes the issue if the approach still relies on mock. I'd be comfortable if the approach uses subclassing.
I'm seeing frequent failures on it - probably it's time to consider disabling the suite?
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] AmplabJenkins removed a comment on pull request #29069: [SPARK-31831][SQL][TESTS] Use subclasses for mock in HiveSessionImplSuite
Posted by GitBox <gi...@apache.org>.
AmplabJenkins removed a comment on pull request #29069:
URL: https://github.com/apache/spark/pull/29069#issuecomment-657163573
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] AmplabJenkins commented on pull request #29069: [SPARK-31831][SQL][TESTS] Use subclasses for mock in HiveSessionImplSuite
Posted by GitBox <gi...@apache.org>.
AmplabJenkins commented on pull request #29069:
URL: https://github.com/apache/spark/pull/29069#issuecomment-657163573
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] frankyin-factual commented on pull request #29069: [SPARK-31831][SQL][TESTS] Use subclasses for mock in HiveSessionImplSuite
Posted by GitBox <gi...@apache.org>.
frankyin-factual commented on pull request #29069:
URL: https://github.com/apache/spark/pull/29069#issuecomment-659139939
I will also take a look.
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] AmplabJenkins removed a comment on pull request #29069: [SPARK-31831][SQL][TESTS] Use subclasses for mock in HiveSessionImplSuite
Posted by GitBox <gi...@apache.org>.
AmplabJenkins removed a comment on pull request #29069:
URL: https://github.com/apache/spark/pull/29069#issuecomment-657151632
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] AmplabJenkins commented on pull request #29069: [SPARK-31831][SQL][TESTS] Use subclasses for mock in HiveSessionImplSuite
Posted by GitBox <gi...@apache.org>.
AmplabJenkins commented on pull request #29069:
URL: https://github.com/apache/spark/pull/29069#issuecomment-657151632
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] frankyin-factual commented on pull request #29069: [SPARK-31831][SQL][TESTS] Use constructor for mock in HiveSessionImplSuite
Posted by GitBox <gi...@apache.org>.
frankyin-factual commented on pull request #29069:
URL: https://github.com/apache/spark/pull/29069#issuecomment-656954852
that's the code for `StdInstantiatorStrategy`:
```
public <T> ObjectInstantiator<T> newInstantiatorOf(Class<T> type) {
if(PlatformDescription.isThisJVM(HOTSPOT) || PlatformDescription.isThisJVM(OPENJDK)) {
// Java 7 GAE was under a security manager so we use a degraded system
if(PlatformDescription.isGoogleAppEngine() && PlatformDescription.SPECIFICATION_VERSION.equals("1.7")) {
if(Serializable.class.isAssignableFrom(type)) {
return new ObjectInputStreamInstantiator<T>(type);
}
return new AccessibleInstantiator<T>(type);
}
// The UnsafeFactoryInstantiator would also work. But according to benchmarks, it is 2.5
// times slower. So I prefer to use this one
return new SunReflectionFactoryInstantiator<T>(type);
}
else if(PlatformDescription.isThisJVM(DALVIK)) {
if(PlatformDescription.isAndroidOpenJDK()) {
// Starting at Android N which is based on OpenJDK
return new UnsafeFactoryInstantiator<T>(type);
}
if(ANDROID_VERSION <= 10) {
// Android 2.3 Gingerbread and lower
return new Android10Instantiator<T>(type);
}
if(ANDROID_VERSION <= 17) {
// Android 3.0 Honeycomb to 4.2 Jelly Bean
return new Android17Instantiator<T>(type);
}
// Android 4.3 until Android N
return new Android18Instantiator<T>(type);
}
else if(PlatformDescription.isThisJVM(JROCKIT)) {
// JRockit is compliant with HotSpot
return new SunReflectionFactoryInstantiator<T>(type);
}
else if(PlatformDescription.isThisJVM(GNU)) {
return new GCJInstantiator<T>(type);
}
else if(PlatformDescription.isThisJVM(PERC)) {
return new PercInstantiator<T>(type);
}
// Fallback instantiator, should work with most modern JVM
return new UnsafeFactoryInstantiator<T>(type);
}
```
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] SparkQA removed a comment on pull request #29069: [SPARK-31831][SQL][TESTS] Use subclasses for mock in HiveSessionImplSuite
Posted by GitBox <gi...@apache.org>.
SparkQA removed a comment on pull request #29069:
URL: https://github.com/apache/spark/pull/29069#issuecomment-657151458
**[Test build #125698 has started](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/125698/testReport)** for PR 29069 at commit [`9067f21`](https://github.com/apache/spark/commit/9067f217c6707da54be56000d25ceda691b38eb6).
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] dongjoon-hyun commented on pull request #29069: [SPARK-31831][SQL][TESTS] Use subclasses for mock in HiveSessionImplSuite
Posted by GitBox <gi...@apache.org>.
dongjoon-hyun commented on pull request #29069:
URL: https://github.com/apache/spark/pull/29069#issuecomment-659113405
Hi, @HeartSaVioR and @frankyin-factual .
This seems to break `master` compilation on Hive 1.2 profile. Could you take a look at the failure?
- https://amplab.cs.berkeley.edu/jenkins/view/Spark%20QA%20Test%20(Dashboard)/job/spark-master-test-maven-hadoop-2.7-hive-1.2/631/
```
[ERROR] [Error] /home/jenkins/workspace/spark-master-test-maven-hadoop-2.7-hive-1.2/sql/hive-thriftserver/src/test/scala/org/apache/spark/sql/hive/thriftserver/HiveSessionImplSuite.scala:30: object rpc is not a member of package org.apache.hive.service
[ERROR] [Error] /home/jenkins/workspace/spark-master-test-maven-hadoop-2.7-hive-1.2/sql/hive-thriftserver/src/test/scala/org/apache/spark/sql/hive/thriftserver/HiveSessionImplSuite.scala:75: not found: type THandleIdentifier
[ERROR] [Error] /home/jenkins/workspace/spark-master-test-maven-hadoop-2.7-hive-1.2/sql/hive-thriftserver/src/test/scala/org/apache/spark/sql/hive/thriftserver/HiveSessionImplSuite.scala:75: not found: type THandleIdentifier
[ERROR] [Error] /home/jenkins/workspace/spark-master-test-maven-hadoop-2.7-hive-1.2/sql/hive-thriftserver/src/test/scala/org/apache/spark/sql/hive/thriftserver/HiveSessionImplSuite.scala:78: not found: type TOperationHandle
[ERROR] [Error] /home/jenkins/workspace/spark-master-test-maven-hadoop-2.7-hive-1.2/sql/hive-thriftserver/src/test/scala/org/apache/spark/sql/hive/thriftserver/HiveSessionImplSuite.scala:78: not found: type TOperationHandle
```
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] SparkQA commented on pull request #29069: [SPARK-31831][SQL][TESTS] Use subclasses for mock in HiveSessionImplSuite
Posted by GitBox <gi...@apache.org>.
SparkQA commented on pull request #29069:
URL: https://github.com/apache/spark/pull/29069#issuecomment-657194434
**[Test build #125713 has started](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/125713/testReport)** for PR 29069 at commit [`1eb2027`](https://github.com/apache/spark/commit/1eb2027d61c231ac96e4a07c485ebe8676b63c08).
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] HeartSaVioR commented on pull request #29069: [SPARK-31831][SQL][TESTS] Use subclasses for mock in HiveSessionImplSuite
Posted by GitBox <gi...@apache.org>.
HeartSaVioR commented on pull request #29069:
URL: https://github.com/apache/spark/pull/29069#issuecomment-657190799
@frankyin-factual
Thanks for the update. The approach looks good. Would you mind if we just put everything in the suite, so that we don't need to find things which is only used at once?
Below change (+ imports) would make you get rid of new java files:
```
class GetCatalogsOperationMock(parentSession: HiveSession)
extends GetCatalogsOperation(parentSession) {
override def runInternal(): Unit = {}
override def getHandle: OperationHandle = {
val uuid: UUID = UUID.randomUUID();
val tHandleIdentifier: THandleIdentifier = new THandleIdentifier()
tHandleIdentifier.setGuid(getByteBufferFromUUID(uuid));
tHandleIdentifier.setSecret(getByteBufferFromUUID(uuid));
val tOperationHandle: TOperationHandle = new TOperationHandle()
tOperationHandle.setOperationId(tHandleIdentifier);
tOperationHandle.setOperationType(TOperationType.GET_TYPE_INFO);
tOperationHandle.setHasResultSetIsSet(false);
new OperationHandle(tOperationHandle);
}
private def getByteBufferFromUUID(uuid: UUID): Array[Byte] = {
val bb: ByteBuffer = ByteBuffer.wrap(new Array[Byte](16))
bb.putLong(uuid.getMostSignificantBits())
bb.putLong(uuid.getLeastSignificantBits())
bb.array();
}
}
class OperationManagerMock extends OperationManager {
private val calledHandles: mutable.Set[OperationHandle] = new mutable.HashSet[OperationHandle]()
override def newGetCatalogsOperation(parentSession: HiveSession): GetCatalogsOperation = {
val operation = new GetCatalogsOperationMock(parentSession)
try {
val m = classOf[OperationManager].getDeclaredMethod("addOperation", classOf[Operation])
m.setAccessible(true)
m.invoke(this, operation)
} catch {
case e@(_: NoSuchMethodException | _: IllegalAccessException |
_: InvocationTargetException) =>
throw new RuntimeException(e)
}
operation
}
override def closeOperation(opHandle: OperationHandle): Unit = {
calledHandles.add(opHandle)
throw new RuntimeException
}
def getCalledHandles: mutable.Set[OperationHandle] = calledHandles
}
```
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] AmplabJenkins commented on pull request #29069: [SPARK-31831][SQL][TESTS] Use subclasses for mock in HiveSessionImplSuite
Posted by GitBox <gi...@apache.org>.
AmplabJenkins commented on pull request #29069:
URL: https://github.com/apache/spark/pull/29069#issuecomment-657152579
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] AmplabJenkins commented on pull request #29069: [SPARK-31831][SQL][TESTS] Use constructor for mock in HiveSessionImplSuite
Posted by GitBox <gi...@apache.org>.
AmplabJenkins commented on pull request #29069:
URL: https://github.com/apache/spark/pull/29069#issuecomment-657150634
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] AmplabJenkins removed a comment on pull request #29069: [SPARK-31831][SQL][TESTS] Use subclasses for mock in HiveSessionImplSuite
Posted by GitBox <gi...@apache.org>.
AmplabJenkins removed a comment on pull request #29069:
URL: https://github.com/apache/spark/pull/29069#issuecomment-657159620
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] AmplabJenkins removed a comment on pull request #29069: [SPARK-31831][SQL][TESTS] Use constructor for mock in HiveSessionImplSuite
Posted by GitBox <gi...@apache.org>.
AmplabJenkins removed a comment on pull request #29069:
URL: https://github.com/apache/spark/pull/29069#issuecomment-657150638
Test FAILed.
Refer to this link for build results (access rights to CI server needed):
https://amplab.cs.berkeley.edu/jenkins//job/SparkPullRequestBuilder/125697/
Test FAILed.
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] AmplabJenkins removed a comment on pull request #29069: [SPARK-31831][SQL][TESTS] Use subclasses for mock in HiveSessionImplSuite
Posted by GitBox <gi...@apache.org>.
AmplabJenkins removed a comment on pull request #29069:
URL: https://github.com/apache/spark/pull/29069#issuecomment-657160470
Test FAILed.
Refer to this link for build results (access rights to CI server needed):
https://amplab.cs.berkeley.edu/jenkins//job/SparkPullRequestBuilder/125701/
Test FAILed.
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] frankyin-factual commented on pull request #29069: [SPARK-31831][SQL][TESTS] Use subclasses for mock in HiveSessionImplSuite
Posted by GitBox <gi...@apache.org>.
frankyin-factual commented on pull request #29069:
URL: https://github.com/apache/spark/pull/29069#issuecomment-659154829
I am working on a combination of 1) and 2). Will push shortly.
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] SparkQA removed a comment on pull request #29069: [SPARK-31831][SQL][TESTS] Use subclasses for mock in HiveSessionImplSuite
Posted by GitBox <gi...@apache.org>.
SparkQA removed a comment on pull request #29069:
URL: https://github.com/apache/spark/pull/29069#issuecomment-657163429
**[Test build #125702 has started](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/125702/testReport)** for PR 29069 at commit [`611ba1b`](https://github.com/apache/spark/commit/611ba1b882cceb8a9657a6b8a175b6aa27aea6b3).
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] AmplabJenkins removed a comment on pull request #29069: [SPARK-31831][SQL][TESTS] Use constructor for mock in HiveSessionImplSuite
Posted by GitBox <gi...@apache.org>.
AmplabJenkins removed a comment on pull request #29069:
URL: https://github.com/apache/spark/pull/29069#issuecomment-656848901
Can one of the admins verify this patch?
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] HeartSaVioR commented on pull request #29069: [SPARK-31831][SQL][TESTS] Use constructor for mock in HiveSessionImplSuite
Posted by GitBox <gi...@apache.org>.
HeartSaVioR commented on pull request #29069:
URL: https://github.com/apache/spark/pull/29069#issuecomment-657028624
I don't know about the details in mockito mock mechanism, but according to the error message and the stack trace, my bet for the problematic spot is `SubclassBytecodeGenerator.mockClass`, and this patch also does go though the path.
I'd say we may need to probably check the thread context classloader just before calling mock, and whether it can find MockAccess. That's why I'd like to just avoid mocking and see manual subclass to just make test work.
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] AmplabJenkins removed a comment on pull request #29069: [SPARK-31831][SQL][TESTS] Use subclasses for mock in HiveSessionImplSuite
Posted by GitBox <gi...@apache.org>.
AmplabJenkins removed a comment on pull request #29069:
URL: https://github.com/apache/spark/pull/29069#issuecomment-657165941
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] HeartSaVioR commented on pull request #29069: [SPARK-31831][SQL][TESTS] Use subclasses for mock in HiveSessionImplSuite
Posted by GitBox <gi...@apache.org>.
HeartSaVioR commented on pull request #29069:
URL: https://github.com/apache/spark/pull/29069#issuecomment-657268918
Thanks! Merged into master.
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] AmplabJenkins removed a comment on pull request #29069: [SPARK-31831][SQL][TESTS] Use constructor for mock in HiveSessionImplSuite
Posted by GitBox <gi...@apache.org>.
AmplabJenkins removed a comment on pull request #29069:
URL: https://github.com/apache/spark/pull/29069#issuecomment-657081269
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] SparkQA commented on pull request #29069: [SPARK-31831][SQL][TESTS] Use constructor for mock in HiveSessionImplSuite
Posted by GitBox <gi...@apache.org>.
SparkQA commented on pull request #29069:
URL: https://github.com/apache/spark/pull/29069#issuecomment-657081095
**[Test build #125686 has started](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/125686/testReport)** for PR 29069 at commit [`c120f0c`](https://github.com/apache/spark/commit/c120f0c1eed34bac71f6569ad72c145b47ac287c).
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] dongjoon-hyun commented on pull request #29069: [SPARK-31831][SQL][TESTS] Use subclasses for mock in HiveSessionImplSuite
Posted by GitBox <gi...@apache.org>.
dongjoon-hyun commented on pull request #29069:
URL: https://github.com/apache/spark/pull/29069#issuecomment-659114303
cc @gatorsmile since he has been interested in `Hive 1.2` profile.
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] AmplabJenkins commented on pull request #29069: [SPARK-31831][SQL][TESTS] Use constructor for mock in HiveSessionImplSuite
Posted by GitBox <gi...@apache.org>.
AmplabJenkins commented on pull request #29069:
URL: https://github.com/apache/spark/pull/29069#issuecomment-657086617
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] SparkQA commented on pull request #29069: [SPARK-31831][SQL][TESTS] Use constructor for mock in HiveSessionImplSuite
Posted by GitBox <gi...@apache.org>.
SparkQA commented on pull request #29069:
URL: https://github.com/apache/spark/pull/29069#issuecomment-657086488
**[Test build #125686 has finished](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/125686/testReport)** for PR 29069 at commit [`c120f0c`](https://github.com/apache/spark/commit/c120f0c1eed34bac71f6569ad72c145b47ac287c).
* This patch passes all tests.
* This patch merges cleanly.
* This patch adds no public classes.
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] AmplabJenkins commented on pull request #29069: [SPARK-31831][SQL][TESTS] Use subclasses for mock in HiveSessionImplSuite
Posted by GitBox <gi...@apache.org>.
AmplabJenkins commented on pull request #29069:
URL: https://github.com/apache/spark/pull/29069#issuecomment-657159620
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] AmplabJenkins commented on pull request #29069: [SPARK-31831][SQL][TESTS] Use constructor for mock in HiveSessionImplSuite
Posted by GitBox <gi...@apache.org>.
AmplabJenkins commented on pull request #29069:
URL: https://github.com/apache/spark/pull/29069#issuecomment-656848901
Can one of the admins verify this patch?
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] dongjoon-hyun commented on pull request #29069: [SPARK-31831][SQL][TESTS] Use subclasses for mock in HiveSessionImplSuite
Posted by GitBox <gi...@apache.org>.
dongjoon-hyun commented on pull request #29069:
URL: https://github.com/apache/spark/pull/29069#issuecomment-659151759
Thank you. I'm fine for all combination (including Hive 2.3 only testing). Please feel free to choose an option. From my side, this also looks not urgent since this is not blocking both GitHub Action and PRBuilder. It has been broken over 3 days already. I hope `Hive 1.2` is going to be removed in the near future eventually after we build a consensus. Sooner is better.
In short, please proceed toward what you think is right, @HeartSaVioR .
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] HeartSaVioR commented on pull request #29069: [SPARK-31831][SQL][TESTS] Use constructor for mock in HiveSessionImplSuite
Posted by GitBox <gi...@apache.org>.
HeartSaVioR commented on pull request #29069:
URL: https://github.com/apache/spark/pull/29069#issuecomment-657018147
ok to 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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] AmplabJenkins commented on pull request #29069: [SPARK-31831][SQL][TESTS] Use constructor for mock in HiveSessionImplSuite
Posted by GitBox <gi...@apache.org>.
AmplabJenkins commented on pull request #29069:
URL: https://github.com/apache/spark/pull/29069#issuecomment-656848455
Can one of the admins verify this patch?
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] frankyin-factual commented on pull request #29069: [SPARK-31831][SQL][TESTS] Use constructor for mock in HiveSessionImplSuite
Posted by GitBox <gi...@apache.org>.
frankyin-factual commented on pull request #29069:
URL: https://github.com/apache/spark/pull/29069#issuecomment-656955342
Turns out our class of mock doesn't have a complicated constructor.
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] HyukjinKwon commented on pull request #29069: [SPARK-31831][SQL][TESTS] Use constructor for mock in HiveSessionImplSuite
Posted by GitBox <gi...@apache.org>.
HyukjinKwon commented on pull request #29069:
URL: https://github.com/apache/spark/pull/29069#issuecomment-657080153
ok to 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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] frankyin-factual commented on pull request #29069: [SPARK-31831][SQL][TESTS] Use subclasses for mock in HiveSessionImplSuite
Posted by GitBox <gi...@apache.org>.
frankyin-factual commented on pull request #29069:
URL: https://github.com/apache/spark/pull/29069#issuecomment-657193575
Just pushed such 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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] SparkQA commented on pull request #29069: [SPARK-31831][SQL][TESTS] Use subclasses for mock in HiveSessionImplSuite
Posted by GitBox <gi...@apache.org>.
SparkQA commented on pull request #29069:
URL: https://github.com/apache/spark/pull/29069#issuecomment-657160461
**[Test build #125701 has finished](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/125701/testReport)** for PR 29069 at commit [`6771504`](https://github.com/apache/spark/commit/6771504eb87c3cf8a6f23f1657c93509c3932041).
* This patch **fails Java style tests**.
* This patch merges cleanly.
* This patch adds the following public classes _(experimental)_:
* `public class GetCatalogsOperationMock extends GetCatalogsOperation `
* `public class OperationManagerMock extends OperationManager `
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] frankyin-factual commented on pull request #29069: [SPARK-31831][SQL][TESTS] Use constructor for mock in HiveSessionImplSuite
Posted by GitBox <gi...@apache.org>.
frankyin-factual commented on pull request #29069:
URL: https://github.com/apache/spark/pull/29069#issuecomment-656954497
I think the default one uses `ObjenesisInstantiator` which uses `StdInstantiatorStrategy` that goes pretty deep in vendor-specific JVM. I switched them to use `ConstructorInstantiator` which just uses plain java reflection to do object instantiation. If the mock `.class` generated has interface `MockAccess`, this is certainly the safest route to construct such instance.
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] frankyin-factual commented on pull request #29069: [SPARK-31831][SQL][TESTS] Use constructor for mock in HiveSessionImplSuite
Posted by GitBox <gi...@apache.org>.
frankyin-factual commented on pull request #29069:
URL: https://github.com/apache/spark/pull/29069#issuecomment-656904754
Still an issue in https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/125621/testReport/org.apache.spark.sql.hive.thriftserver/HiveSessionImplSuite/_It_is_not_a_test_it_is_a_sbt_testing_SuiteSelector_/
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] AmplabJenkins removed a comment on pull request #29069: [SPARK-31831][SQL][TESTS] Use subclasses for mock in HiveSessionImplSuite
Posted by GitBox <gi...@apache.org>.
AmplabJenkins removed a comment on pull request #29069:
URL: https://github.com/apache/spark/pull/29069#issuecomment-657152579
Merged build finished. Test FAILed.
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] HyukjinKwon commented on pull request #29069: [SPARK-31831][SQL][TESTS] Use subclasses for mock in HiveSessionImplSuite
Posted by GitBox <gi...@apache.org>.
HyukjinKwon commented on pull request #29069:
URL: https://github.com/apache/spark/pull/29069#issuecomment-659132336
Let's revert if it's non trivial to fix @HeartSaVioR.
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] AmplabJenkins commented on pull request #29069: [SPARK-31831][SQL][TESTS] Use subclasses for mock in HiveSessionImplSuite
Posted by GitBox <gi...@apache.org>.
AmplabJenkins commented on pull request #29069:
URL: https://github.com/apache/spark/pull/29069#issuecomment-657197562
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] SparkQA removed a comment on pull request #29069: [SPARK-31831][SQL][TESTS] Use constructor for mock in HiveSessionImplSuite
Posted by GitBox <gi...@apache.org>.
SparkQA removed a comment on pull request #29069:
URL: https://github.com/apache/spark/pull/29069#issuecomment-657081095
**[Test build #125686 has started](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/125686/testReport)** for PR 29069 at commit [`c120f0c`](https://github.com/apache/spark/commit/c120f0c1eed34bac71f6569ad72c145b47ac287c).
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] frankyin-factual commented on pull request #29069: [SPARK-31831][SQL][TESTS] Use constructor for mock in HiveSessionImplSuite
Posted by GitBox <gi...@apache.org>.
frankyin-factual commented on pull request #29069:
URL: https://github.com/apache/spark/pull/29069#issuecomment-657150400
Yeah, I changed to use the subclasses for mocking. I think it is a classloader issue with threading.
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] AmplabJenkins removed a comment on pull request #29069: [SPARK-31831][SQL][TESTS] Use subclasses for mock in HiveSessionImplSuite
Posted by GitBox <gi...@apache.org>.
AmplabJenkins removed a comment on pull request #29069:
URL: https://github.com/apache/spark/pull/29069#issuecomment-657194726
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] SparkQA commented on pull request #29069: [SPARK-31831][SQL][TESTS] Use subclasses for mock in HiveSessionImplSuite
Posted by GitBox <gi...@apache.org>.
SparkQA commented on pull request #29069:
URL: https://github.com/apache/spark/pull/29069#issuecomment-657151458
**[Test build #125698 has started](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/125698/testReport)** for PR 29069 at commit [`9067f21`](https://github.com/apache/spark/commit/9067f217c6707da54be56000d25ceda691b38eb6).
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] frankyin-factual commented on pull request #29069: [SPARK-31831][SQL][TESTS] Use subclasses for mock in HiveSessionImplSuite
Posted by GitBox <gi...@apache.org>.
frankyin-factual commented on pull request #29069:
URL: https://github.com/apache/spark/pull/29069#issuecomment-659157488
@HeartSaVioR @dongjoon-hyun https://github.com/apache/spark/pull/29129
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] dongjoon-hyun commented on a change in pull request #29069: [SPARK-31831][SQL][TESTS] Use subclasses for mock in HiveSessionImplSuite
Posted by GitBox <gi...@apache.org>.
dongjoon-hyun commented on a change in pull request #29069:
URL: https://github.com/apache/spark/pull/29069#discussion_r455467438
##########
File path: sql/hive-thriftserver/src/test/scala/org/apache/spark/sql/hive/thriftserver/HiveSessionImplSuite.scala
##########
@@ -16,28 +16,30 @@
*/
package org.apache.spark.sql.hive.thriftserver
+import java.lang.reflect.InvocationTargetException
+import java.nio.ByteBuffer
+import java.util.UUID
+
import scala.collection.JavaConverters._
+import scala.collection.mutable
import org.apache.hadoop.hive.conf.HiveConf
import org.apache.hive.service.cli.OperationHandle
-import org.apache.hive.service.cli.operation.{GetCatalogsOperation, OperationManager}
-import org.apache.hive.service.cli.session.{HiveSessionImpl, SessionManager}
-import org.mockito.Mockito.{mock, verify, when}
-import org.mockito.invocation.InvocationOnMock
+import org.apache.hive.service.cli.operation.{GetCatalogsOperation, Operation, OperationManager}
+import org.apache.hive.service.cli.session.{HiveSession, HiveSessionImpl, SessionManager}
+import org.apache.hive.service.rpc.thrift.{THandleIdentifier, TOperationHandle, TOperationType}
Review comment:
According to the compilation error, this `import` is invalid in Hive 1.2.
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] HeartSaVioR closed pull request #29069: [SPARK-31831][SQL][TESTS] Use subclasses for mock in HiveSessionImplSuite
Posted by GitBox <gi...@apache.org>.
HeartSaVioR closed pull request #29069:
URL: https://github.com/apache/spark/pull/29069
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] AmplabJenkins removed a comment on pull request #29069: [SPARK-31831][SQL][TESTS] Use constructor for mock in HiveSessionImplSuite
Posted by GitBox <gi...@apache.org>.
AmplabJenkins removed a comment on pull request #29069:
URL: https://github.com/apache/spark/pull/29069#issuecomment-657086617
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] SparkQA commented on pull request #29069: [SPARK-31831][SQL][TESTS] Use subclasses for mock in HiveSessionImplSuite
Posted by GitBox <gi...@apache.org>.
SparkQA commented on pull request #29069:
URL: https://github.com/apache/spark/pull/29069#issuecomment-657165900
**[Test build #125702 has finished](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/125702/testReport)** for PR 29069 at commit [`611ba1b`](https://github.com/apache/spark/commit/611ba1b882cceb8a9657a6b8a175b6aa27aea6b3).
* This patch passes all tests.
* This patch merges cleanly.
* This patch adds the following public classes _(experimental)_:
* `public class GetCatalogsOperationMock extends GetCatalogsOperation `
* `public class OperationManagerMock extends OperationManager `
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] SparkQA commented on pull request #29069: [SPARK-31831][SQL][TESTS] Use subclasses for mock in HiveSessionImplSuite
Posted by GitBox <gi...@apache.org>.
SparkQA commented on pull request #29069:
URL: https://github.com/apache/spark/pull/29069#issuecomment-657159524
**[Test build #125701 has started](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/125701/testReport)** for PR 29069 at commit [`6771504`](https://github.com/apache/spark/commit/6771504eb87c3cf8a6f23f1657c93509c3932041).
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] dongjoon-hyun commented on pull request #29069: [SPARK-31831][SQL][TESTS] Use subclasses for mock in HiveSessionImplSuite
Posted by GitBox <gi...@apache.org>.
dongjoon-hyun commented on pull request #29069:
URL: https://github.com/apache/spark/pull/29069#issuecomment-657240325
Thanks for working on this, @HeartSaVioR and @frankyin-factual .
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] SparkQA removed a comment on pull request #29069: [SPARK-31831][SQL][TESTS] Use subclasses for mock in HiveSessionImplSuite
Posted by GitBox <gi...@apache.org>.
SparkQA removed a comment on pull request #29069:
URL: https://github.com/apache/spark/pull/29069#issuecomment-657194434
**[Test build #125713 has started](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/125713/testReport)** for PR 29069 at commit [`1eb2027`](https://github.com/apache/spark/commit/1eb2027d61c231ac96e4a07c485ebe8676b63c08).
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] HeartSaVioR edited a comment on pull request #29069: [SPARK-31831][SQL][TESTS] Use subclasses for mock in HiveSessionImplSuite
Posted by GitBox <gi...@apache.org>.
HeartSaVioR edited a comment on pull request #29069:
URL: https://github.com/apache/spark/pull/29069#issuecomment-659144203
I guess we have several possible approaches here:
1. place the suite to the Hive-version specific directory (with new config on pom.xml to add the test source based on version)
2. create shim and place the shim to the Hive-version specific directory (same)
3. revert and try to place the suite on the list of running in separate JVM (less chance to encounter classloader issue)
I guess the straightforward approach would be 1 - I guess it should work smoothly, though if we want to ensure the suite runs in both versions we'll end up with duplicating codes. If that doesn't matter much we can just do that, or even we can just enable the test on hive 2.3 only (as the suite is technically irrelevant to Hive 1.2 vs Hive 2.3).
Less redundant but more complicated would be 2. I'm not 100% sure how much complexity is needed to make a shim for the suite, and not sure it worths to do instead of simply allowing redundant codes a bit.
The simplest approach would be 3, but it's not guaranteed to fix the flakiness. I'd say it'd be better to leave as the last resort.
WDYT?
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] AmplabJenkins removed a comment on pull request #29069: [SPARK-31831][SQL][TESTS] Use subclasses for mock in HiveSessionImplSuite
Posted by GitBox <gi...@apache.org>.
AmplabJenkins removed a comment on pull request #29069:
URL: https://github.com/apache/spark/pull/29069#issuecomment-657197562
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] AmplabJenkins removed a comment on pull request #29069: [SPARK-31831][SQL][TESTS] Use constructor for mock in HiveSessionImplSuite
Posted by GitBox <gi...@apache.org>.
AmplabJenkins removed a comment on pull request #29069:
URL: https://github.com/apache/spark/pull/29069#issuecomment-656848455
Can one of the admins verify this patch?
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] AmplabJenkins removed a comment on pull request #29069: [SPARK-31831][SQL][TESTS] Use constructor for mock in HiveSessionImplSuite
Posted by GitBox <gi...@apache.org>.
AmplabJenkins removed a comment on pull request #29069:
URL: https://github.com/apache/spark/pull/29069#issuecomment-657150634
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] AmplabJenkins removed a comment on pull request #29069: [SPARK-31831][SQL][TESTS] Use subclasses for mock in HiveSessionImplSuite
Posted by GitBox <gi...@apache.org>.
AmplabJenkins removed a comment on pull request #29069:
URL: https://github.com/apache/spark/pull/29069#issuecomment-657152582
Test FAILed.
Refer to this link for build results (access rights to CI server needed):
https://amplab.cs.berkeley.edu/jenkins//job/SparkPullRequestBuilder/125698/
Test FAILed.
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] SparkQA commented on pull request #29069: [SPARK-31831][SQL][TESTS] Use subclasses for mock in HiveSessionImplSuite
Posted by GitBox <gi...@apache.org>.
SparkQA commented on pull request #29069:
URL: https://github.com/apache/spark/pull/29069#issuecomment-657163429
**[Test build #125702 has started](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/125702/testReport)** for PR 29069 at commit [`611ba1b`](https://github.com/apache/spark/commit/611ba1b882cceb8a9657a6b8a175b6aa27aea6b3).
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] AmplabJenkins commented on pull request #29069: [SPARK-31831][SQL][TESTS] Use constructor for mock in HiveSessionImplSuite
Posted by GitBox <gi...@apache.org>.
AmplabJenkins commented on pull request #29069:
URL: https://github.com/apache/spark/pull/29069#issuecomment-657150686
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] SparkQA removed a comment on pull request #29069: [SPARK-31831][SQL][TESTS] Use constructor for mock in HiveSessionImplSuite
Posted by GitBox <gi...@apache.org>.
SparkQA removed a comment on pull request #29069:
URL: https://github.com/apache/spark/pull/29069#issuecomment-657150528
**[Test build #125697 has started](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/125697/testReport)** for PR 29069 at commit [`7c668fe`](https://github.com/apache/spark/commit/7c668fe60ed37570465880fb8b778fb3a9dd5449).
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] SparkQA commented on pull request #29069: [SPARK-31831][SQL][TESTS] Use subclasses for mock in HiveSessionImplSuite
Posted by GitBox <gi...@apache.org>.
SparkQA commented on pull request #29069:
URL: https://github.com/apache/spark/pull/29069#issuecomment-657197498
**[Test build #125713 has finished](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/125713/testReport)** for PR 29069 at commit [`1eb2027`](https://github.com/apache/spark/commit/1eb2027d61c231ac96e4a07c485ebe8676b63c08).
* This patch passes all tests.
* This patch merges cleanly.
* This patch adds the following public classes _(experimental)_:
* `class GetCatalogsOperationMock(parentSession: HiveSession)`
* `class OperationManagerMock extends OperationManager `
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] HeartSaVioR commented on pull request #29069: [SPARK-31831][SQL][TESTS] Use subclasses for mock in HiveSessionImplSuite
Posted by GitBox <gi...@apache.org>.
HeartSaVioR commented on pull request #29069:
URL: https://github.com/apache/spark/pull/29069#issuecomment-659144203
I guess we have several possible approaches here:
1. place the suite to the Hive-version specific directory (with new config on pom.xml to add the test source based on version)
2. create shim and place the shim to the Hive-version specific directory (same)
3. revert and try to place the suite on the list of running in separate JVM (less chance to encounter classloader issue)
I guess the straightforward approach would be 1 - I guess it should work smoothly, though if we want to ensure the suite runs in both versions we'll end up with duplicating codes. If that doesn't matter much we can just do that, or even we can just enable the test on hive 2.3 only (as the suite is technically irrelevant to Hive 1.2 vs Hive 2.3).
Less redundant but more complicated would be 2. I'm not 100% sure how much complexity is needed to make a shim for the suite, and it worths to do instead of simply allowing redundant codes a bit.
The simplest approach would be 3, but it's not guaranteed to fix the flakiness. I'd say it'd be better to leave as the last resort.
WDYT?
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] SparkQA commented on pull request #29069: [SPARK-31831][SQL][TESTS] Use constructor for mock in HiveSessionImplSuite
Posted by GitBox <gi...@apache.org>.
SparkQA commented on pull request #29069:
URL: https://github.com/apache/spark/pull/29069#issuecomment-657150528
**[Test build #125697 has started](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/125697/testReport)** for PR 29069 at commit [`7c668fe`](https://github.com/apache/spark/commit/7c668fe60ed37570465880fb8b778fb3a9dd5449).
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] SparkQA commented on pull request #29069: [SPARK-31831][SQL][TESTS] Use subclasses for mock in HiveSessionImplSuite
Posted by GitBox <gi...@apache.org>.
SparkQA commented on pull request #29069:
URL: https://github.com/apache/spark/pull/29069#issuecomment-657152573
**[Test build #125698 has finished](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/125698/testReport)** for PR 29069 at commit [`9067f21`](https://github.com/apache/spark/commit/9067f217c6707da54be56000d25ceda691b38eb6).
* This patch **fails to build**.
* This patch merges cleanly.
* This patch adds the following public classes _(experimental)_:
* `public class GetCatalogsOperationMock extends GetCatalogsOperation `
* `public class OperationManagerMock extends OperationManager `
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] SparkQA commented on pull request #29069: [SPARK-31831][SQL][TESTS] Use constructor for mock in HiveSessionImplSuite
Posted by GitBox <gi...@apache.org>.
SparkQA commented on pull request #29069:
URL: https://github.com/apache/spark/pull/29069#issuecomment-657150629
**[Test build #125697 has finished](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/125697/testReport)** for PR 29069 at commit [`7c668fe`](https://github.com/apache/spark/commit/7c668fe60ed37570465880fb8b778fb3a9dd5449).
* This patch **fails RAT tests**.
* This patch merges cleanly.
* This patch adds the following public classes _(experimental)_:
* `public class GetCatalogsOperationMock extends GetCatalogsOperation `
* `public class OperationManagerMock extends OperationManager `
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] AmplabJenkins commented on pull request #29069: [SPARK-31831][SQL][TESTS] Use subclasses for mock in HiveSessionImplSuite
Posted by GitBox <gi...@apache.org>.
AmplabJenkins commented on pull request #29069:
URL: https://github.com/apache/spark/pull/29069#issuecomment-657160466
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] AmplabJenkins removed a comment on pull request #29069: [SPARK-31831][SQL][TESTS] Use subclasses for mock in HiveSessionImplSuite
Posted by GitBox <gi...@apache.org>.
AmplabJenkins removed a comment on pull request #29069:
URL: https://github.com/apache/spark/pull/29069#issuecomment-657160466
Merged build finished. Test FAILed.
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] HeartSaVioR commented on pull request #29069: [SPARK-31831][SQL][TESTS] Use subclasses for mock in HiveSessionImplSuite
Posted by GitBox <gi...@apache.org>.
HeartSaVioR commented on pull request #29069:
URL: https://github.com/apache/spark/pull/29069#issuecomment-659138098
I'll take a look at it today. We can revert it if there's no good way to fix.
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] dongjoon-hyun commented on pull request #29069: [SPARK-31831][SQL][TESTS] Use constructor for mock in HiveSessionImplSuite
Posted by GitBox <gi...@apache.org>.
dongjoon-hyun commented on pull request #29069:
URL: https://github.com/apache/spark/pull/29069#issuecomment-656923626
cc @HeartSaVioR
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] AmplabJenkins commented on pull request #29069: [SPARK-31831][SQL][TESTS] Use subclasses for mock in HiveSessionImplSuite
Posted by GitBox <gi...@apache.org>.
AmplabJenkins commented on pull request #29069:
URL: https://github.com/apache/spark/pull/29069#issuecomment-657165941
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] AmplabJenkins commented on pull request #29069: [SPARK-31831][SQL][TESTS] Use subclasses for mock in HiveSessionImplSuite
Posted by GitBox <gi...@apache.org>.
AmplabJenkins commented on pull request #29069:
URL: https://github.com/apache/spark/pull/29069#issuecomment-657194726
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org