mesos-reviews mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From Armand Grillet <agril...@mesosphere.io>
Subject Re: Review Request 68951: Updated verify-reviews.py to use current interpreter in subprocesses.
Date Mon, 08 Oct 2018 18:23:40 GMT


> On Oct. 8, 2018, 8:12 p.m., Till Toenshoff wrote:
> > support/verify-reviews.py
> > Line 97 (original), 97 (patched)
> > <https://reviews.apache.org/r/68951/diff/1/?file=2095334#file2095334line97>
> >
> >     IIUC, then `sys.executable` may be `None`. Shall we guard against that?

As described in https://docs.python.org/3/library/sys.html#sys.executable: "If Python is unable
to retrieve the real path to its executable, sys.executable will be an empty string or None.".
In the first case, "", the shebang in the first line of `support/apply-reviews.py` (https://github.com/apache/mesos/blob/8375e426d1c07f625ee18cd439e0dd4f1dc804c5/support/apply-reviews.py#L1)
will make us use Python 3 AFAIK. 

The reason why I set the current interpreter instead of nothing is that a user might specify
an interpreter when running `support/apply-reviews.py`. This is not the first time we do this
in our support scripts: https://github.com/apache/mesos/blob/8375e426d1c07f625ee18cd439e0dd4f1dc804c5/support/mesos-style.py#L291


- Armand


-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/68951/#review209317
-----------------------------------------------------------


On Oct. 8, 2018, 8:06 p.m., Armand Grillet wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/68951/
> -----------------------------------------------------------
> 
> (Updated Oct. 8, 2018, 8:06 p.m.)
> 
> 
> Review request for mesos, Till Toenshoff and Vinod Kone.
> 
> 
> Bugs: MESOS-9253
>     https://issues.apache.org/jira/browse/MESOS-9253
> 
> 
> Repository: mesos
> 
> 
> Description
> -------
> 
> This changes the command used in `support/verify-reviews.py` when
> running `support/apply-reviews.py` as a subprocess. It was previously
> `"python"`, which is generally Python 2, and is now `sys.executable`.
> 
> That way, if verify-reviews.py is run with Python 3 (as it should),
> apply-reviews.py will be run with the same Python 3 interpreter. This
> should fix the `ImportError` issues we have recently seen in our CI.
> 
> 
> Diffs
> -----
> 
>   support/verify-reviews.py 56321ae65b38a1c62f5589b6a8aaa3993fa3dd5f 
> 
> 
> Diff: https://reviews.apache.org/r/68951/diff/1/
> 
> 
> Testing
> -------
> 
> I have created a simple `test.py` file to check that the interpreter was correctly found:
> 
> ```
> import sys
> 
> 
> def apply_review(review_id):
>     """Apply a review using the script apply-reviews.py."""
>     print("Applying review %s" % review_id)
>     print("%s support/apply-reviews.py -n -r %s" % (sys.executable, review_id))
> 
> apply_review(1337)
> apply_review("1337")
> ```
> 
> In both cases, `python3 test.py` prints `/usr/local/bin/python3 support/apply-reviews.py
-n -r 1337` which is what I expected.
> 
> 
> Thanks,
> 
> Armand Grillet
> 
>


Mime
  • Unnamed multipart/alternative (inline, None, 0 bytes)
View raw message