Skip to content

Conversation

@echoTomei
Copy link
Contributor

No description provided.

@andrewor14
Copy link
Contributor

ok to test

@andrewor14
Copy link
Contributor

Good catch, this change looks fine.

@SparkQA
Copy link

SparkQA commented Dec 21, 2015

Test build #48125 has finished for PR 10407 at commit 7959c1f.

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

@andrewor14
Copy link
Contributor

@echoTomei it's failing style tests because there's a whitespace at the end of the line

@echoTomei
Copy link
Contributor Author

@andrewor14 Sorry for my careless.I'll create another PR.

…uccessfull.

Delete the white space at the end of the line.
@SparkQA
Copy link

SparkQA commented Dec 22, 2015

Test build #48151 has finished for PR 10407 at commit 3a38c14.

  • This patch fails to build.
  • This patch merges cleanly.
  • This patch adds no public classes.

@echoTomei
Copy link
Contributor Author

@SparkQA The build error shows that the function cancel() is not a member of [java.util.concurrent.ScheduledFuture[_]] . I check the interface ScheduledFuture API ,confirming that this interface has cancel() function. So I don't understand why occur this error. Can you give me some suggestion?Thanks.

@andrewor14
Copy link
Contributor

retest this please

@SparkQA
Copy link

SparkQA commented Dec 22, 2015

Test build #48208 has finished for PR 10407 at commit 3a38c14.

  • This patch fails to build.
  • This patch merges cleanly.
  • This patch adds no public classes.

@andrewor14
Copy link
Contributor

That is really confusing... @srowen do you know if we're missing something obvious?

@srowen
Copy link
Member

srowen commented Dec 22, 2015

Actually the error says that the var is an AtomicReference and this does not have a cancel method. Indeed, it does in master, after e1bcf6a

Is this PR against a quite old version of master? while it doesn't technically need a rebase, it really does.

@zsxwing
Copy link
Member

zsxwing commented Dec 22, 2015

I think a better approach is changing registrationRetryThread.scheduleAtFixedRate to registrationRetryThread.schedule.

@andrewor14
Copy link
Contributor

I think a better approach is changing registrationRetryThread.scheduleAtFixedRate to registrationRetryThread.schedule.

+1. Right now we call it both recursively and iteratively, which doesn't make sense...

@echoTomei
Copy link
Contributor Author

It seemed that change to registrationRetryThread.schedule is a better way.

It is no need to register to master iteratively.
@zsxwing
Copy link
Member

zsxwing commented Dec 23, 2015

Could you rebase on the latest master?

@SparkQA
Copy link

SparkQA commented Dec 23, 2015

Test build #48231 has finished for PR 10407 at commit 93c005d.

  • This patch fails R style tests.
  • This patch does not merge cleanly.
  • This patch adds no public classes.

@andrewor14
Copy link
Contributor

Can you close this PR now that you've opened another one #10447

@echoTomei echoTomei closed this Dec 25, 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