Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fixes SPARK-8730 - Deser objects containing a primitive class attribute #7122

Closed
wants to merge 1 commit into from

Conversation

EugenCepoi
Copy link
Contributor

No description provided.

@SparkQA
Copy link

SparkQA commented Jun 30, 2015

Test build #36127 has finished for PR 7122 at commit 66dbc80.

  • This patch fails Scala style tests.
  • This patch merges cleanly.
  • This patch adds no public classes.

@SparkQA
Copy link

SparkQA commented Jun 30, 2015

Test build #36128 has finished for PR 7122 at commit 42244d6.

  • This patch passes all tests.
  • This patch merges cleanly.
  • This patch adds no public classes.

@@ -63,13 +63,30 @@ private[spark] class JavaDeserializationStream(in: InputStream, loader: ClassLoa

private val objIn = new ObjectInputStream(in) {
override def resolveClass(desc: ObjectStreamClass): Class[_] =
Class.forName(desc.getName, false, loader)
try Class.forName(desc.getName, false, loader)
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

wrap the try block w/ braces eg

try {
   Class.forName(...)
} catch {
 ...

@SparkQA
Copy link

SparkQA commented Jul 24, 2015

Test build #38351 has finished for PR 7122 at commit 9cfc942.

  • This patch passes all tests.
  • This patch merges cleanly.
  • This patch adds no public classes.

}

def readObject[T: ClassTag](): T = objIn.readObject().asInstanceOf[T]
def close() { objIn.close() }
}

private object JavaSerDe {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

can you rename this to match the class, JavaDeserializationStream? Alternatively we could give it some name which is more specific, eg. JavaSerializationPrimitiveClassHelper or something, but I think just making it a companion object is good enough.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

good point, I hesitated to do it at first but don't remember why I decided to not put it in a companion object. Will do

@SparkQA
Copy link

SparkQA commented Jul 24, 2015

Test build #38394 has finished for PR 7122 at commit 24e3d99.

  • This patch fails Spark unit tests.
  • This patch merges cleanly.
  • This patch adds no public classes.

@EugenCepoi
Copy link
Contributor Author

@squito I don't get what fails. Is it some random issue or did I really break something? If I didn't should we just re run the tests?

@SparkQA
Copy link

SparkQA commented Aug 31, 2015

Test build #41828 has finished for PR 7122 at commit 5e50c62.

  • This patch passes all tests.
  • This patch merges cleanly.
  • This patch adds no public classes.

@squito
Copy link
Contributor

squito commented Aug 31, 2015

@EugenCepoi yikes, sorry I forgot about this for so long! thanks for brining it up to date, merging to master

@asfgit asfgit closed this in 72f6dbf Aug 31, 2015
@EugenCepoi
Copy link
Contributor Author

Np, thanks for the review :)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
3 participants