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

Add pipelining to podman connection plugin #57579

Open
wants to merge 1 commit into
base: devel
from

Conversation

Projects
None yet
4 participants
@jordemort
Copy link

commented Jun 8, 2019

SUMMARY

This PR adds pipelining support to the podman connection plugin.

ISSUE TYPE
  • Feature Pull Request
COMPONENT NAME

plugins/connection/podman.py

ADDITIONAL INFORMATION

All that was missing here was to pass in_data over to _podman, and to set has_pipelining to True. cc @TomasTomecek

@ghost

This comment has been minimized.

Copy link

commented Jun 9, 2019

@jordemort hi,I am sorry to bother you, and I am same to you as a contributor. All checks have passed for our submitted code, but how to merge it to devel branch or waiting repository manager merges it?

@jordemort

This comment has been minimized.

Copy link
Author

commented Jun 9, 2019

@Ansible912 I'm new here myself, I don't know how they do things

@TomasTomecek

This comment has been minimized.

Copy link
Contributor

commented Jun 9, 2019

Wow, what a simple fix. Actually, while trying this out, I discovered an issue that if podman mount fails, ansible continues execution. #57584

@Ansible912 @jordemort we need to wait for someone with commit access to approve and merge (to devel), e.g. @abadger or @mattclay or @sivel

could this fix land in 2.8.z?

@TomasTomecek
Copy link
Contributor

left a comment

LGTM, tested locally using a simple playbook (didn't observe almost any speed-up since the playbook was really trivial)

@jordemort

This comment has been minimized.

Copy link
Author

commented Jun 9, 2019

@TomasTomecek The buildah plugin has the same problem, which I fixed in #57552 along with doing all the other stuff, but if you think it's worthwhile / could be backported to a 2.8.x, I could split that off into a separate PR similar to this one. Probably wouldn't want to do all of 57552 in a patch release but the pipelining fix is just one line.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
You can’t perform that action at this time.