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

cloud_tests: emit dots on Travis while fetching images #347

Merged
merged 3 commits into from
May 14, 2020

Conversation

OddBloke
Copy link
Collaborator

@OddBloke OddBloke commented May 4, 2020

This ensures that Travis will not kill our tests if fetching images is
taking a long time.

In implementation turns, this introduces a context manager which will
spin up a multiprocessing.Process in the background and print a dot to
stdout every 10 seconds. The process is terminated when the context
manager exits.

@OddBloke OddBloke force-pushed the travis branch 4 times, most recently from b3c8764 to 408d03b Compare May 6, 2020 15:55
This ensures that Travis will not kill our tests if fetching images is
taking a long time.

In implementation terms, this introduces a context manager which will
spin up a multiprocessing.Process in the background and print a dot to
stdout every 10 seconds.  The process is terminated when the context
manager exits.

This also drop the use of travis_wait, which was being used to work
around this issue.
@OddBloke
Copy link
Collaborator Author

OddBloke commented May 6, 2020

We can see in https://travis-ci.org/github/canonical/cloud-init/jobs/683935711 that dots are emitted as expected (search for "acquiring image for"), this is ready for review.

Copy link
Contributor

@lucasmoura lucasmoura left a comment

Choose a reason for hiding this comment

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

Just a consideration that in the PR description it is said that the code will emit a dot every 60 seconds, while the actual code emits a dot every 10s. I don't know if that is what we want, the 10s wait, but it it is, no problem.

@OddBloke
Copy link
Collaborator Author

Good catch, thanks Lucas!

@OddBloke OddBloke requested a review from blackboxsw May 12, 2020 15:21
Copy link
Collaborator

@blackboxsw blackboxsw left a comment

Choose a reason for hiding this comment

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

+1 here. needs rebase and merge, but otherwise good.

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.

3 participants