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 execution times in lxd 3.0.1 and add pipelining support to execute #321

Merged
merged 1 commit into from
Aug 16, 2018

Conversation

lasizoillo
Copy link
Contributor

Fix issue #316 and issue #244

It works (tox and integration tests) with lxd versions 3.0.1 and 2.0.11

Can be tested with this script to view some new features:

from pylxd import Client
import io
import logging

logger = logging.getLogger(__name__)
handler = logging.StreamHandler()
handler.setFormatter(
    logging.Formatter("%(relativeCreated)6d %(threadName)s %(message)s")
)
logger.addHandler(handler)
logger.setLevel(logging.INFO)

c = Client()
cont = c.containers.all()[0]
if cont.status != "Running":
    cont.start(wait=True)

# Stdin demo
logger.info(cont.execute(["cat", "-"], stdin_payload=open(__file__)).stdout)
logger.info(cont.execute(["cat", "-"], stdin_payload="hola"))
logger.info(cont.execute(["cat", "-"], stdin_payload=b"hola"))
logger.info(cont.execute(["cat", "-"], stdin_payload=io.StringIO("hola")))
logger.info(cont.execute(["cat", "-"], stdin_payload=io.BytesIO(b"hola")))


# Stdout demo
def handle_out(msg):
    logger.info(msg)


cont.execute(
    [
        "bash", "-c",
        'while read  c; do echo "$c"; sleep 1; done < /etc/hosts'
    ], stdout_handler=handle_out
)

container.execute is retro-compatible but adds pipeline features. Now you can communicate with your commands via stdin and manage output while command is executing.

@codecov-io
Copy link

codecov-io commented Jul 13, 2018

Codecov Report

Merging #321 into master will decrease coverage by <.01%.
The diff coverage is 100%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master     #321      +/-   ##
==========================================
- Coverage   96.84%   96.84%   -0.01%     
==========================================
  Files          11       11              
  Lines         920      918       -2     
  Branches      106      106              
==========================================
- Hits          891      889       -2     
  Misses         10       10              
  Partials       19       19
Impacted Files Coverage Δ
pylxd/models/container.py 91.06% <100%> (-0.08%) ⬇️

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 0d5c227...b5feace. Read the comment docs.

@lasizoillo lasizoillo force-pushed the BetterExecute branch 2 times, most recently from 258e377 to c6f5ab5 Compare July 13, 2018 14:46
@ajkavanagh
Copy link
Contributor

Thanks for putting this together. However, there seems to be something very odd going on with the PR as it includes a bunch of already committed patches? Perhaps you need a git fetch/merge and/or rebase from this master to only submit the PR that is relevant to this change?

I'll do some testing with the branch/commit from your repo as well. Thanks very much.
(Sorry for the delay; I've been on holiday and away).

Copy link
Contributor

@ajkavanagh ajkavanagh left a comment

Choose a reason for hiding this comment

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

Needs rebasing (I think) as it has many already committed changes in the PR.

Signed-off-by: lasizoillo <lasizoillo@gmail.com>
@lasizoillo
Copy link
Contributor Author

Rebased

@Silentphantom62
Copy link

@ajkavanagh Can you please review this? Have been waiting on a fix for this for a while.

@ajkavanagh
Copy link
Contributor

@Silentphantom62 and @lasizoillo -- sorry for the delay; just got back from holiday -- I'll review & do some integration tests today.

@ajkavanagh
Copy link
Contributor

Okay, I've tested this in 1604 and 1804 hosts and containers, and everything seems to work great. Thanks very much for the PR, and sorry for the delay in getting it merged.

@Silentphantom62
Copy link

Thanks @ajkavanagh what's the tentative release date of this fix?

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.

5 participants