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

Optionally check for presence of pv and use - feedback sought #86

Open
wants to merge 1 commit into
base: flatcar-master
Choose a base branch
from

Conversation

nhi-vanye
Copy link

optionally use PV to provide feedback while writing to disk

If we find pv is installed on the system running flatcar-install, use it to report writing imagefile to disk

How to use

Install from an image file (not a URL)

This is for feedback - I've only fixed the path I'm using (install from file), happy to extend this to the download path if requested.

Screenshot 2023-01-13 at 10 47 56 am

If we find pv is installed on the system running flatcar-install,
use it to report writing images to disk
@@ -572,9 +586,9 @@ function install_from_file() {

echo "Writing ${IMAGE_FILE}..."
if [[ "${IMAGE_FILE}" =~ \.bz2$ ]]; then
${BZIP_UTIL} -cd "${IMAGE_FILE}" | write_to_disk
${have_pv:-cat} "${IMAGE_FILE}" | ${BZIP_UTIL} -cd | write_to_disk
Copy link
Member

@pothos pothos Jan 13, 2023

Choose a reason for hiding this comment

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

Thanks for your PR!
Could we do | "${have_pv:-cat}" | write_do_disk to show the same stats in both cases?

Copy link
Author

@nhi-vanye nhi-vanye Jan 13, 2023

Choose a reason for hiding this comment

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

it seems pv needs to be the first stage in the pipeline in order to include an ETA; when its streaming between stdin and out it doesn't know the total size...

Fitting it into the other places would obviously be easier in stream mode...

So I guess its code simplicity (streaming) vs features (first in pipeline) - I'm not tied to having an ETA so do you have a preferred approach ?

@pothos
Copy link
Member

pothos commented Jan 13, 2023

Maybe we should also limit the use of pv to interactive terminals? Otherwise it's spamming the log output, I guess?

@pothos
Copy link
Member

pothos commented Jan 13, 2023

Or make it opt-in via an extra flag?

@nhi-vanye
Copy link
Author

Maybe we should also limit the use of pv to interactive terminals? Otherwise it's spamming the log output, I guess?

Doh! Yes will do.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
3 participants