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

Log transferring #1051

Merged
merged 1 commit into from
Jun 10, 2021
Merged

Log transferring #1051

merged 1 commit into from
Jun 10, 2021

Conversation

alexcb
Copy link
Contributor

@alexcb alexcb commented Jun 8, 2021

log files which are sent to buildkit

When running in verbose mode, log which files are sent to buildkit including file stats and files which are excluded (ignored).

In regular (non-verbose) mode, the progressbars have been replaced with a message displaying total bytes sent:

image

and under verbose mode, it displays all files:

image

The progress bar had to be removed as it was overwriting the final output of text while running in verbose mode, and to be able to programatically disable it for only verbose mode would have required a fair amount of changes to the buildkit code.

This depends on earthly/buildkit#44

@alexcb alexcb force-pushed the log-transferring branch 6 times, most recently from 7a2e241 to 042f738 Compare June 9, 2021 20:38
@alexcb alexcb marked this pull request as ready for review June 9, 2021 21:14
@@ -11,7 +11,7 @@ buildkitd:
ARG BUILDKIT_BASE_IMAGE=$BUILDKIT_PROJECT+build
END
ELSE
ARG BUILDKIT_BASE_IMAGE=github.com/earthly/buildkit:c896a903b73a001a756c8d40330874451dbe38c4+build
ARG BUILDKIT_BASE_IMAGE=github.com/earthly/buildkit:907351dab34de3d22aeebea7a03593473ab7a1f0+build
Copy link
Contributor Author

Choose a reason for hiding this comment

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

I will update this once earthly/buildkit#44 is merged.

@alexcb alexcb requested review from vladaionescu and dchw June 9, 2021 21:15
@vladaionescu
Copy link
Member

I wonder if the file stat updates can be made less verbose. For example by printing how many files+dirs were stat'd per tree, instead of listing every single file.

@alexcb
Copy link
Contributor Author

alexcb commented Jun 9, 2021

I wonder if the file stat updates can be made less verbose. For example by printing how many files+dirs were stat'd per tree, instead of listing every single file.

We would need some additional changes to fsutils to differentiate between files and dirs, but we could count the total number of stats per walk.

Another possibility would be to change -V to handle multiple Vs which would increase the verbosity level, so at -V we could just skip printing stats, and at -VV we would show them. (edit... urfav/cli doesn't support this currently, there's a PR to add it urfave/cli#1257 )

@vladaionescu
Copy link
Member

We would need some additional changes to fsutils to differentiate between files and dirs, but we could count the total number of stats per walk.

Yeah, total number would be great.

I'd avoid multiple levels of verbosity, to keep things simple.

@alexcb
Copy link
Contributor Author

alexcb commented Jun 9, 2021

this is a little tidier:

image

@alexcb alexcb force-pushed the log-transferring branch 8 times, most recently from 293ca7b to 2fd5dd3 Compare June 9, 2021 23:55
@vladaionescu
Copy link
Member

That looks really good

@alexcb alexcb force-pushed the log-transferring branch from 2fd5dd3 to a1013d8 Compare June 10, 2021 00:06
@alexcb
Copy link
Contributor Author

alexcb commented Jun 10, 2021

another small tweak for the text:

image

Copy link
Member

@vladaionescu vladaionescu 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 nit

buildcontext/local.go Show resolved Hide resolved
When running in verbose mode, log which files are sent to buildkit
including file stats and files which are excluded (ignored).

Signed-off-by: Alex Couture-Beil <alex@earthly.dev>
@alexcb alexcb force-pushed the log-transferring branch from a1013d8 to 8c5f513 Compare June 10, 2021 00:49
@alexcb alexcb enabled auto-merge (squash) June 10, 2021 01:04
@alexcb alexcb merged commit 70decc7 into main Jun 10, 2021
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.

2 participants