Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

FIX: correction to popen call returning bytestream #401

Merged
merged 1 commit into from
Jul 24, 2020

Conversation

barbequesauce
Copy link
Collaborator

Per #400 Thank you @SenorSmartyPants

Copy link

@SenorSmartyPants SenorSmartyPants left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks like it should fix it to me. Ready to try it out.

@SenorSmartyPants
Copy link

All Popen calls should be updated.

@evilhero
Copy link
Collaborator

evilhero commented Jul 23, 2020

No need to update any Popen calls. This is a logger error, pure & simple. Modifying the line:
logger.fdebug("Script result: " + out)
to:
logger.fdebug("Script result: %s" % out)
would resolve it.

I should also mention that while the above logger line would fix it, adding text=true shouldn't do anything to harm any part of the post-processing module as far as I'm aware (not sure if that modifies some of the return bytes that we check later, but w/e). But again, the other places we use Popen have already been tested, and work as intended - so no need to change those in case the results are modified which causes code changes down the line.

@SenorSmartyPants
Copy link

I'm cool with whichever fix that gets implemented. Just want to write more post processing scripts and have them run. Thanks for all your work.

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.

None yet

3 participants