Skip to content

Conversation

@Sephiroth-Lin
Copy link
Contributor

In driver now will start a server socket and use a wildcard ip, use 127.0.0.0 is more reasonable, as we only use it by local Python process.
/cc @davies

@Sephiroth-Lin Sephiroth-Lin changed the title Specify ip of python server scoket [SPARK-6604][PySpark]Specify ip of python server scoket Mar 30, 2015
@WangTaoTheTonic
Copy link
Contributor

ok to test

@WangTaoTheTonic
Copy link
Contributor

Jenkins, test this please.

@davies
Copy link
Contributor

davies commented Mar 30, 2015

LGTM

@SparkQA
Copy link

SparkQA commented Mar 30, 2015

Test build #631 has finished for PR 5256 at commit c88bee9.

  • This patch passes all tests.
  • This patch merges cleanly.
  • This patch adds no public classes.
  • This patch does not change any dependencies.

@JoshRosen
Copy link
Contributor

There was some previous discussion of localhost vs 127.0.0.1 at #3425 and I think the conclusion there was against this change. @davies, do your comments there about 127.0.0.1 sometimes being not portable (e.g. IPv6-only machines) still apply here?

@davies
Copy link
Contributor

davies commented Apr 2, 2015

@JoshRosen Changing the bind address to localhost is reasonable, but we have not figured out a good way to specify local interface, localhost or 127.0.0.1 both have PRO and CON, either one is better than another, so I against to change localhost to 127.0.0.1 without good reason.

I'd vote +1 for * -> 127.0.0.1 change, -0 for localhost -> 127.0.0.1.

@srowen
Copy link
Member

srowen commented Apr 3, 2015

(Related discussion was #3425 ^D^D^D which you already found. Can this use "localhost" for consistency with https://docs.python.org/2/library/socketserver.html ? FWIW the current code looks equivalent to passing InetAddress.anyLocalAddress() which is different indeed from localhost.)

@srowen
Copy link
Member

srowen commented Apr 15, 2015

@Sephiroth-Lin can you rebase this? and does it make sense to refer to "localhost" instead? Consider also that we have Utils.localHostName() which is an attempt to standardize what is meant by "the address of the local machine"

@Sephiroth-Lin
Copy link
Contributor Author

@srowen OK, thanks.

@srowen
Copy link
Member

srowen commented Apr 16, 2015

LGTM pending tests.

@SparkQA
Copy link

SparkQA commented Apr 16, 2015

Test build #685 has finished for PR 5256 at commit 7b3c633.

  • This patch fails Spark unit tests.
  • This patch merges cleanly.
  • This patch adds no public classes.
  • This patch adds the following new dependencies:
    • commons-math3-3.4.1.jar
    • snappy-java-1.1.1.7.jar
  • This patch removes the following dependencies:
    • commons-math3-3.1.1.jar
    • snappy-java-1.1.1.6.jar

@Sephiroth-Lin
Copy link
Contributor Author

Jenkins, retest this please.

@SparkQA
Copy link

SparkQA commented Apr 17, 2015

Test build #688 has finished for PR 5256 at commit 7b3c633.

  • This patch fails Spark unit tests.
  • This patch merges cleanly.
  • This patch adds no public classes.
  • This patch does not change any dependencies.

@srowen
Copy link
Member

srowen commented Apr 17, 2015

Test failure looks unrelated as it's not in Python, but let me try one more time.

@srowen
Copy link
Member

srowen commented Apr 17, 2015

Jenkins, retest this please

@SparkQA
Copy link

SparkQA commented Apr 17, 2015

Test build #30474 has finished for PR 5256 at commit 7b3c633.

  • This patch passes all tests.
  • This patch merges cleanly.
  • This patch adds no public classes.
  • This patch does not change any dependencies.

@asfgit asfgit closed this in dc48ba9 Apr 17, 2015
@Sephiroth-Lin Sephiroth-Lin deleted the SPARK-6604 branch May 15, 2016 10:15
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.

6 participants