Skip to content

Conversation

@prabeesh
Copy link
Contributor

@prabeesh prabeesh commented Jan 8, 2015

modified to adhere to accepted coding standards as pointed by @tdas in PR #3844

@SparkQA
Copy link

SparkQA commented Jan 8, 2015

Test build #25214 has started for PR 3947 at commit 5786340.

  • This patch merges cleanly.

@srowen
Copy link
Member

srowen commented Jan 8, 2015

I'm not sure about most of these.

  • Why is tempDir a field?
  • _ is more idiomatic in Scala than null for a reference field's default value.
  • Why remove the call to disconnect? almost seems like it should be in a finally block
  • The regex change alters the functionality. I think "word count" examples across the world split on space.
  • It's probably fine to resume one MqttMessage int he loop but are you sure?

@SparkQA
Copy link

SparkQA commented Jan 8, 2015

Test build #25214 has finished for PR 3947 at commit 5786340.

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

@AmplabJenkins
Copy link

Test FAILed.
Refer to this link for build results (access rights to CI server needed):
https://amplab.cs.berkeley.edu/jenkins//job/SparkPullRequestBuilder/25214/
Test FAILed.

@prabeesh
Copy link
Contributor Author

prabeesh commented Jan 8, 2015

  • the hard coded '\tmp' may create garbage. so created temp directory using internal methods and deleting after the use.
  • revert to _
  • added the finally block
  • revert to " "
  • In while loop, the MqttMessage object is getting created so many times. here just send same messages repeatedly

@SparkQA
Copy link

SparkQA commented Jan 8, 2015

Test build #25222 has started for PR 3947 at commit 0cc5e46.

  • This patch merges cleanly.

@srowen
Copy link
Member

srowen commented Jan 8, 2015

@prabeesh my question about /tmp was why it is not a local variable. About MqttMessage, my question is simply whether you know you can reuse the message object. I imagine you can, but it is not obvious.

@SparkQA
Copy link

SparkQA commented Jan 8, 2015

Test build #25223 has started for PR 3947 at commit fc6abd5.

  • This patch merges cleanly.

@SparkQA
Copy link

SparkQA commented Jan 8, 2015

Test build #25222 has finished for PR 3947 at commit 0cc5e46.

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

@AmplabJenkins
Copy link

Test PASSed.
Refer to this link for build results (access rights to CI server needed):
https://amplab.cs.berkeley.edu/jenkins//job/SparkPullRequestBuilder/25222/
Test PASSed.

@SparkQA
Copy link

SparkQA commented Jan 8, 2015

Test build #25223 has finished for PR 3947 at commit fc6abd5.

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

@AmplabJenkins
Copy link

Test PASSed.
Refer to this link for build results (access rights to CI server needed):
https://amplab.cs.berkeley.edu/jenkins//job/SparkPullRequestBuilder/25223/
Test PASSed.

@SparkQA
Copy link

SparkQA commented Jan 8, 2015

Test build #25236 has started for PR 3947 at commit 8185c3b.

  • This patch merges cleanly.

@SparkQA
Copy link

SparkQA commented Jan 8, 2015

Test build #25236 has finished for PR 3947 at commit 8185c3b.

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

@AmplabJenkins
Copy link

Test PASSed.
Refer to this link for build results (access rights to CI server needed):
https://amplab.cs.berkeley.edu/jenkins//job/SparkPullRequestBuilder/25236/
Test PASSed.

@prabeesh prabeesh changed the title some trivial fixes [minor]some trivial fixes Jan 9, 2015
@prabeesh prabeesh changed the title [minor]some trivial fixes [Minor]some trivial fixes Jan 9, 2015
Copy link
Contributor

Choose a reason for hiding this comment

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

Its more scala-like to not have the type in case of a val. So
val peristance = new MqttDefaultFi....

Also, peristance --> persistence !!!!

@tdas
Copy link
Contributor

tdas commented Jan 9, 2015

I am okay not having a jira about this. But please update the title to have [streaming] and MQTT streaming example in it. Some trivial fixes makes hard for us to understand from the commit list about what this patch was about.

@prabeesh prabeesh changed the title [Minor]some trivial fixes [Minor][streaming][MQTT streaming example] some trivial fixes Jan 9, 2015
@SparkQA
Copy link

SparkQA commented Jan 9, 2015

Test build #25318 has started for PR 3947 at commit 3a66ae2.

  • This patch merges cleanly.

@SparkQA
Copy link

SparkQA commented Jan 9, 2015

Test build #25319 has started for PR 3947 at commit e350661.

  • This patch merges cleanly.

@SparkQA
Copy link

SparkQA commented Jan 9, 2015

Test build #25318 has finished for PR 3947 at commit 3a66ae2.

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

@AmplabJenkins
Copy link

Test PASSed.
Refer to this link for build results (access rights to CI server needed):
https://amplab.cs.berkeley.edu/jenkins//job/SparkPullRequestBuilder/25318/
Test PASSed.

@SparkQA
Copy link

SparkQA commented Jan 9, 2015

Test build #25319 has finished for PR 3947 at commit e350661.

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

@AmplabJenkins
Copy link

Test PASSed.
Refer to this link for build results (access rights to CI server needed):
https://amplab.cs.berkeley.edu/jenkins//job/SparkPullRequestBuilder/25319/
Test PASSed.

@prabeesh prabeesh closed this Jan 23, 2015
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants