0

I'm trying to execute a script via Python. This script runs a toolset called review board tools based on the arguments passed into the script.

When I run the command:

yes | bash review 'feature-signature' 'PR_Reviewers' 'myrepo' 'myuser' 'mypassword' 'update' 'development' 'PR user' 'PR summary https://myjira.atlassian.net/browse/NEB-4257' 'https://myjira.atlassian.net/browse/NEB-4257' 'PR description'

The shell gets caught because rbtools asks the users on update if they want to update the review requests when the update is run.

When I run this command exactly as is in my terminal minus the obfuscation for my credentials and actual information it succeeds and finishes the update with no issues. It pipes the yes | into the script and auto answers the yes prompt.

The execution only hangs in Python.

The bash command review is a script I wrote to call review board tools I have tried putting the yes | in that script as well to no avail.

#!/usr/bin/env bash

BRANCH_NAME=$1
REVIEWERS=$2
REPO=$3
rb_user=$4
rb_password=$5
action=$6
base_branch=$7

#added these options
submit_as=$8
summary=$9
jira_id=$10
desc=$11

echo "Jira Link: $jira_id"
echo "Desc: $desc"
echo "Repo: $repo"
echo "Submit as: $submit_as"
echo "Summary $summary"
echo "Branch name $BRANCH_NAME"


cd $REPO
git checkout $BRANCH_NAME

if [ "$action" == "post" ]; then

    rbt post --repository=$REPO \
                   --tracking-branch="origin/$BRANCH_NAME" \
                   --parent="origin/$base_branch" \
                   --branch="$BRANCH_NAME" \
                   --guess-fields=auto \
                   --target-groups=$REVIEWERS \
                   --publish \
                   --server="http://reviews.domain" \
                   --username="$rb_user" \
                   --password=$rb_password \
                   --submit-as="$submit_as" \
                   --bugs-closed="$jira_id" \
                   --description="$summary" \
                   -d
else
   yes | rbt post --update --repository=$REPO \
                   --tracking-branch="origin/$BRANCH_NAME" \
                   --parent="origin/$base_branch" \
                   --branch="$BRANCH_NAME" \
                   --guess-fields="auto" \
                   --target-groups="$REVIEWERS" \
                   --publish \
                   --server="http://reviews.domain" \
                   --username="$rb_user" \
                   --password="$rb_password" \
                   --submit-as="$submit_as" \
                   --bugs-closed="$jira_id" \
                   --description="$summary" \
                   -d
fi

cd ../
exit 0

My Python code that executes the subprocess command is:

def run_command(command):
    p = subprocess.Popen(command, stdout=subprocess.PIPE, stderr=subprocess.PIPE, shell=True)
    out, err = p.communicate()
    if err:
        if "bash" in command:
            return err
        else:
            print err
    return out

I don't understand why when I execute this command from Python the script hangs and gets stuck on the update, but when I run the exact command I print out in my Python script and run it manually in terminal it doesn't hang and completes with no issue.

I understand what's happening with the Python as p.communicate is waiting for the process to exist. The weird part is I get an email and the review requests get updated but my script never continues. Thus, it would see as though the yes is being piped into the command and its running no issues, but Python hangs like it didn't run or exit properly.

Unfortunately, rbtools doesn't have an auto yes function and thus I have to pipe the yes with a shell.

Any ideas on how to resolve this issue?

martineau
  • 119,623
  • 25
  • 170
  • 301
Grant Zukel
  • 1,153
  • 2
  • 24
  • 48
  • You've got a `yes |` inside the script as well -- why do you have a second one on the outside? – Charles Duffy Dec 17 '18 at 20:46
  • Note too that well-behaved software generally doesn't prompt at all when its stdin is connected to a non-TTY source (such as `/dev/null`). If your `rbt` program doesn't do that, maybe report it to upstream as a feature request (that they either use defaults or allow environment variables to be used to specify prompt results when not attached to a TTY)? – Charles Duffy Dec 17 '18 at 20:47
  • ...btw, if *you're* the author of `rbt`, you can use `if [ -t 0 ]; then ...prompt here...; else ...use default...; fi` to implement such logic; `-t` checks whether the given FD (0 for stdin) is connected to a TTY, which will be false if run with `subprocess.Popen(..., stdin=subprocess.DEVNULL)`, or ` – Charles Duffy Dec 17 '18 at 20:49
  • ...or with a program written in Python, one can/should make that prompting conditional on [`os.isatty(0)`](https://docs.python.org/3/library/os.html#os.isatty`) returning False. Filed a corresponding feature request with RBTools @ https://hellosplat.com/s/beanbag/tickets/4773/ – Charles Duffy Dec 17 '18 at 20:55
  • @CharlesDuffy because neither work I was just trying things. I'm not the author of review board tools, but I will make an issue on their github because I agree they are not handling it correctly. I'm just trying to find a workaround for now. – Grant Zukel Dec 17 '18 at 21:50
  • Thanks for posting that up just clicked the link you provided. – Grant Zukel Dec 17 '18 at 21:54
  • also when I say rbt I mean review board tools, its the common tool set you can install with review board they list on their site to utilize command line to do things with reviewboard. They also have a python sdk but it doens't seem to be able to do the auto fields. I might just write it completely in python if there is no hack to get around the TTY problem. – Grant Zukel Dec 17 '18 at 21:56
  • doing rb_command = ("yes | bash review '%s' '%s' '%s' '%s' '%s' '%s' '%s' '%s' '%s' '%s' &" % (branch_being_merged, repo, review_board_user, review_board_password, "update", origin_branch,author_username, summary, jira_link, desc)) os.system(rb_command) solves my immediate issue, but I can't get the output which is ok just kinda annoying. – Grant Zukel Dec 17 '18 at 22:21
  • From a security perspective, by the way, that code is extremely unsafe. Consider what happens if a description or summary contains `'"$(rm -rf ~)"'` (with the literal single quotes). You're much, **much** better off avoiding `shell=True` altogether, and passing an argv array: `['bash', 'review', branch_being_merged, repo, review_board_user, review_board_password, 'update', origin_branch, author_username, summary, jira_link, desc], stdin=subprocess.PIPE`, and then passing `communicate()` a string that answers your prompt. – Charles Duffy Dec 17 '18 at 22:22
  • I agree, I will most likely just go through the code for review board tools tomorrow and fix it. With that method the process stays running even though it actually completes. – Grant Zukel Dec 17 '18 at 22:58
  • If `p.communicate()` says it's still running, that means it started *something* that inherited a handle on its stdout or stderr (so it can't finish reading from the read end of the pipeline until all the copies of the write end are closed). Look at what processes are left running that hold the other side of the pipeline, and that'll be your answer. – Charles Duffy Dec 17 '18 at 23:05
  • @CharlesDuffy I added more information on that feature request you started to help them diagnose the issue. I will see if I can solve it tomorrow by going to rbtools code. – Grant Zukel Dec 18 '18 at 00:10
  • I actually don't think that was helpful -- the feature request was very tightly scoped; the additional information widens it from a feature request to a support request, which requires much more effort to process, and is thus more likely to be closed without having any changes enacted. – Charles Duffy Dec 18 '18 at 14:01

0 Answers0