[GitHub] spark pull request #19998: [SPARK-22813][BUILD] Use lsof or /usr/sbin/lsof i...
Github user asfgit closed the pull request at: https://github.com/apache/spark/pull/19998 --- - To unsubscribe, e-mail: reviews-unsubscr...@spark.apache.org For additional commands, e-mail: reviews-h...@spark.apache.org
[GitHub] spark pull request #19998: [SPARK-22813][BUILD] Use lsof or /usr/sbin/lsof i...
Github user kiszk commented on a diff in the pull request:
https://github.com/apache/spark/pull/19998#discussion_r157334828
--- Diff: dev/run-tests.py ---
@@ -253,9 +253,11 @@ def kill_zinc_on_port(zinc_port):
"""
Kill the Zinc process running on the given port, if one exists.
"""
-cmd = ("/usr/sbin/lsof -P |grep %s | grep LISTEN "
- "| awk '{ print $2; }' | xargs kill") % zinc_port
-subprocess.check_call(cmd, shell=True)
+cmd = "%s -P |grep %s | grep LISTEN | awk '{ print $2; }' | xargs kill"
--- End diff --
Great catch, I also check it.
```
>>> print(which("lsof"))
/usr/bin/lsof
>>>
% ls /usr/bin/lsof /usr/sbin/lsof
ls: cannot access '/usr/sbin/lsof': No such file or directory
/usr/bin/lsof
```
---
-
To unsubscribe, e-mail: reviews-unsubscr...@spark.apache.org
For additional commands, e-mail: reviews-h...@spark.apache.org
[GitHub] spark pull request #19998: [SPARK-22813][BUILD] Use lsof or /usr/sbin/lsof i...
Github user HyukjinKwon commented on a diff in the pull request:
https://github.com/apache/spark/pull/19998#discussion_r157333856
--- Diff: dev/run-tests.py ---
@@ -253,9 +253,11 @@ def kill_zinc_on_port(zinc_port):
"""
Kill the Zinc process running on the given port, if one exists.
"""
-cmd = ("/usr/sbin/lsof -P |grep %s | grep LISTEN "
- "| awk '{ print $2; }' | xargs kill") % zinc_port
-subprocess.check_call(cmd, shell=True)
+cmd = "%s -P |grep %s | grep LISTEN | awk '{ print $2; }' | xargs kill"
--- End diff --
Ah, @kiszk, I think we can actually use
`sparktestsupport.shellutils.which("...")` too like what we do for java:
https://github.com/apache/spark/blob/964e5ff22cefe336cd47d3a9309a8d1428b476b6/dev/run-tests.py#L153
So, like ..
```python
cmd = "%s -P |grep %s | grep LISTEN | awk '{ print $2; }' | xargs kill"
lsof_exe = which("lsof")
subprocess.check_call(cmd % (lsof_exe if lsof_exe else "/usr/sbin/lsof",
zinc_port), shell=True)
```
I just double checked:
```
>>> lsof_exe = which("lsof")
>>> cmd % (lsof_exe if lsof_exe else "/usr/sbin/lsof", zinc_port)
"/usr/sbin/lsof -P |grep 1234 | grep LISTEN | awk '{ print $2; }' | xargs
kill"
>>> lsof_exe = which("lsof")
>>> cmd % (lsof_exe if lsof_exe else "/usr/bin/lsof", zinc_port)
"/usr/sbin/lsof -P |grep 1234 | grep LISTEN | awk '{ print $2; }' | xargs
kill"
```
```
>>> lsof_exe = which("foo")
>>> cmd % (lsof_exe if lsof_exe else "/usr/sbin/lsof", zinc_port)
"/usr/sbin/lsof -P |grep 1234 | grep LISTEN | awk '{ print $2; }' | xargs
kill"
>>> lsof_exe = which("bar")
>>> cmd % (lsof_exe if lsof_exe else "/usr/bin/lsof", zinc_port)
"/usr/bin/lsof -P |grep 1234 | grep LISTEN | awk '{ print $2; }' | xargs
kill"
```
---
-
To unsubscribe, e-mail: reviews-unsubscr...@spark.apache.org
For additional commands, e-mail: reviews-h...@spark.apache.org
[GitHub] spark pull request #19998: [SPARK-22813][BUILD] Use lsof or /usr/sbin/lsof i...
Github user kiszk commented on a diff in the pull request:
https://github.com/apache/spark/pull/19998#discussion_r157333278
--- Diff: dev/run-tests.py ---
@@ -253,9 +253,11 @@ def kill_zinc_on_port(zinc_port):
"""
Kill the Zinc process running on the given port, if one exists.
"""
-cmd = ("/usr/sbin/lsof -P |grep %s | grep LISTEN "
- "| awk '{ print $2; }' | xargs kill") % zinc_port
-subprocess.check_call(cmd, shell=True)
+cmd = "%s -P |grep %s | grep LISTEN | awk '{ print $2; }' | xargs kill"
+try:
+subprocess.check_call(cmd % ("lsof", zinc_port), shell=True)
+except:
+subprocess.call(cmd % ("/usr/sbin/lsof", zinc_port), shell=True)
--- End diff --
I see. Since this change is not strong preference, I will revert this
change to keep the original behavior.
---
-
To unsubscribe, e-mail: reviews-unsubscr...@spark.apache.org
For additional commands, e-mail: reviews-h...@spark.apache.org
[GitHub] spark pull request #19998: [SPARK-22813][BUILD] Use lsof or /usr/sbin/lsof i...
Github user HyukjinKwon commented on a diff in the pull request:
https://github.com/apache/spark/pull/19998#discussion_r157333189
--- Diff: dev/run-tests.py ---
@@ -253,9 +253,11 @@ def kill_zinc_on_port(zinc_port):
"""
Kill the Zinc process running on the given port, if one exists.
"""
-cmd = ("/usr/sbin/lsof -P |grep %s | grep LISTEN "
- "| awk '{ print $2; }' | xargs kill") % zinc_port
-subprocess.check_call(cmd, shell=True)
+cmd = "%s -P |grep %s | grep LISTEN | awk '{ print $2; }' | xargs kill"
+try:
+subprocess.check_call(cmd % ("lsof", zinc_port), shell=True)
+except:
+subprocess.call(cmd % ("/usr/sbin/lsof", zinc_port), shell=True)
--- End diff --
Hm, but it changes what originally `kill_zinc_on_port` does though because
now it is not guaranteed to kill it. I see the point but let's stick to the
original behaviour.
---
-
To unsubscribe, e-mail: reviews-unsubscr...@spark.apache.org
For additional commands, e-mail: reviews-h...@spark.apache.org
[GitHub] spark pull request #19998: [SPARK-22813][BUILD] Use lsof or /usr/sbin/lsof i...
Github user kiszk commented on a diff in the pull request:
https://github.com/apache/spark/pull/19998#discussion_r157333050
--- Diff: dev/run-tests.py ---
@@ -253,9 +253,11 @@ def kill_zinc_on_port(zinc_port):
"""
Kill the Zinc process running on the given port, if one exists.
"""
-cmd = ("/usr/sbin/lsof -P |grep %s | grep LISTEN "
- "| awk '{ print $2; }' | xargs kill") % zinc_port
-subprocess.check_call(cmd, shell=True)
+cmd = "%s -P |grep %s | grep LISTEN | awk '{ print $2; }' | xargs kill"
+try:
+subprocess.check_call(cmd % ("lsof", zinc_port), shell=True)
+except:
+subprocess.call(cmd % ("/usr/sbin/lsof", zinc_port), shell=True)
--- End diff --
I intentionally use `subprocess.call` to continue the execution even if
`lsof` and `/usr/sbin/lsof` do not exist. This is because it is ok for other
steps if we fail to kill `zinc`.
WDYT?
---
-
To unsubscribe, e-mail: reviews-unsubscr...@spark.apache.org
For additional commands, e-mail: reviews-h...@spark.apache.org
[GitHub] spark pull request #19998: [SPARK-22813][BUILD] Use lsof or /usr/sbin/lsof i...
Github user HyukjinKwon commented on a diff in the pull request:
https://github.com/apache/spark/pull/19998#discussion_r157332995
--- Diff: dev/run-tests.py ---
@@ -253,9 +253,11 @@ def kill_zinc_on_port(zinc_port):
"""
Kill the Zinc process running on the given port, if one exists.
"""
-cmd = ("/usr/sbin/lsof -P |grep %s | grep LISTEN "
- "| awk '{ print $2; }' | xargs kill") % zinc_port
-subprocess.check_call(cmd, shell=True)
+cmd = "%s -P |grep %s | grep LISTEN | awk '{ print $2; }' | xargs kill"
+try:
+subprocess.check_call(cmd % ("lsof", zinc_port), shell=True)
+except:
+subprocess.call(cmd % ("/usr/sbin/lsof", zinc_port), shell=True)
--- End diff --
Maybe, `subprocess.call` -> `subprocess.check_call`?
---
-
To unsubscribe, e-mail: reviews-unsubscr...@spark.apache.org
For additional commands, e-mail: reviews-h...@spark.apache.org
[GitHub] spark pull request #19998: [SPARK-22813][BUILD] Use lsof or /usr/sbin/lsof i...
Github user kiszk commented on a diff in the pull request:
https://github.com/apache/spark/pull/19998#discussion_r157332103
--- Diff: dev/run-tests.py ---
@@ -253,9 +253,14 @@ def kill_zinc_on_port(zinc_port):
"""
Kill the Zinc process running on the given port, if one exists.
"""
-cmd = ("/usr/sbin/lsof -P |grep %s | grep LISTEN "
- "| awk '{ print $2; }' | xargs kill") % zinc_port
-subprocess.check_call(cmd, shell=True)
+try:
+cmd = ("lsof -P |grep %s | grep LISTEN "
+ "| awk '{ print $2; }' | xargs kill") % zinc_port
+subprocess.check_call(cmd, shell=True)
+except:
--- End diff --
Yes, if the command does not exist, an exception occurs. Thus, we can
execute one of the two cases.
Yea, to use `cmd` is fine.
---
-
To unsubscribe, e-mail: reviews-unsubscr...@spark.apache.org
For additional commands, e-mail: reviews-h...@spark.apache.org
[GitHub] spark pull request #19998: [SPARK-22813][BUILD] Use lsof or /usr/sbin/lsof i...
Github user HyukjinKwon commented on a diff in the pull request:
https://github.com/apache/spark/pull/19998#discussion_r157331922
--- Diff: dev/run-tests.py ---
@@ -253,9 +253,14 @@ def kill_zinc_on_port(zinc_port):
"""
Kill the Zinc process running on the given port, if one exists.
"""
-cmd = ("/usr/sbin/lsof -P |grep %s | grep LISTEN "
- "| awk '{ print $2; }' | xargs kill") % zinc_port
-subprocess.check_call(cmd, shell=True)
+try:
+cmd = ("lsof -P |grep %s | grep LISTEN "
+ "| awk '{ print $2; }' | xargs kill") % zinc_port
+subprocess.check_call(cmd, shell=True)
+except:
--- End diff --
Could we catch the explicit exception?
Also, I think we could this like:
```python
cmd = "%s -P |grep %s | grep LISTEN | awk '{ print $2; }' | xargs kill"
...
lsof = "lsof"
subprocess.check_call(cmd % (lsof zinc_port), shell=True)
...
lsof = "/usr/sbin/lsof"
subprocess.check_call(cmd % (lsof zinc_port), shell=True)
```
---
-
To unsubscribe, e-mail: reviews-unsubscr...@spark.apache.org
For additional commands, e-mail: reviews-h...@spark.apache.org
