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 a tail_file context manager to the Host class #211

Merged
merged 1 commit into from
May 11, 2023

Conversation

JacobCallahan
Copy link
Member

@JacobCallahan JacobCallahan commented May 8, 2023

This is a pretty common request and should be useful in a number of scenarios. Usage is pretty simple:

    with my_host.session.tail_file("/var/log/messages") as res:
        # do something that creates new messages
    print(res.stdout)

@JacobCallahan JacobCallahan added the enhancement New feature or request label May 8, 2023
@JacobCallahan
Copy link
Member Author

I was debating whether this should be on the host object itself or on the host's session object. Let me know what you think. The alternative would look like this

    with my_host.session.tail_file("/var/log/messages") as res:
        # do something that creates new messages
    print(res.stdout)

This is a pretty common request and should be useful in a number of
scenarios. Usage is pretty simple:

    with my_host.session.tail_file("/var/log/messages") as res:
        # do something that creates new messages
    print(res.stdout)
@JacobCallahan
Copy link
Member Author

@rplevka the new version is in now. The downside I forgot about is that it needed to be duplicated for the ContainerSession, but that's not a big deal.

Copy link
Collaborator

@Griffin-Sullivan Griffin-Sullivan left a comment

Choose a reason for hiding this comment

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

============== 2 passed in 457.36s (0:07:37) =============

Tests are passing. Very excited to have this in for testing! One minor gripe is that the broker_settings.yaml file is hard coded in the tests, so I had to copy it to my local dir. We should consider changing the tests to look for settings in ~/.broker

@JacobCallahan
Copy link
Member Author

@Griffin-Sullivan great idea! Is that something you're wanting to do or would you prefer I handle that in this PR?

@Griffin-Sullivan
Copy link
Collaborator

I can do it in a separate PR.

@JacobCallahan JacobCallahan merged commit 74e9d6b into SatelliteQE:master May 11, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants