add: output improvements

[casperdcl]

dvc add output improvements

• slight wording updates
• add progress for stage.create() (cleared upon completion)
• leave some stats behind?
• fix failing tests
• fix md3sum progress not clearing properly
• progress-aware logger.info
  • fix/update tests
• To track the changes with git, run: prints once rather than per-file
• Computing md5 for a large number of files. This is only done once. message replaced with Computing md5 for a large number of files inside cleared away progress bars
• Saving information to '*.dvc'. messages removed (since files are already mentioned in final commit reminder)
  • make sure all other calls to stage.dump() do not need this verbosity

old:

new:

related

• exception handling #2545
• add: output improvements #2546 (comment)

for future issue/PR

• Add stats #2658

[codecov]

Codecov Report

Merging #2546 into master will decrease coverage by 0.01%.
The diff coverage is 100%.

@@            Coverage Diff             @@
##           master    #2546      +/-   ##
==========================================
- Coverage   92.35%   92.34%   -0.02%     
==========================================
  Files         135      135              
  Lines        8111     8126      +15     
==========================================
+ Hits         7491     7504      +13     
- Misses        620      622       +2

Impacted Files	Coverage Δ
dvc/exceptions.py	99.14% <ø> (ø)	⬆️
dvc/repo/add.py	98.07% <100%> (-1.93%)	⬇️
dvc/remote/base.py	93.58% <100%> (-0.03%)	⬇️
dvc/logger.py	88.6% <100%> (+0.44%)	⬆️
dvc/command/base.py	96.55% <100%> (ø)	⬆️
dvc/command/add.py	100% <100%> (ø)	⬆️
dvc/stage.py	96.44% <100%> (ø)	⬆️
dvc/version.py	83.33% <0%> (-2.78%)	⬇️

---

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 3667830...f02951f. Read the comment docs.

[efiop]

@casperdcl Please use your forked repo to work on such PRs. We have some stuff to improve until we allow in-upstream branches, since travis runs two checks and one always fails. Plus it also helps keep the upstream tidy :)

[casperdcl]

@efiop here's a recording of before and after. It highlights some remaining issues.

1. there was a problem with md3sum progress not clearing properly (happens e.g. at 0:50) which I've now fixed
2. logger.info is not progress-aware (logger.debug, logger.error are aware). This needs fixing (happens e.g. at 2:40)
3. To track the changes with git, run: really should print at the end with a list of dvc files rather than one at a time with lots of empty newlines.
4. Still need to add some stats

[casperdcl]

@efiop new version:

[casperdcl]

main problem is dvc.repo.scm_context.scm_context has a repo.scm.remind_to_track() which should ideally be collected and delayed until after all adds get executed.

[efiop]

@casperdcl You can simply make dvc/repo/add.py accept multiple targets and then process them accordingly, instead of doing that on CLI level in dvc/command/add.py. That way even scm_context will work properly automatically.

[casperdcl]

new version with only final commit reminder message:

[casperdcl]

@efiop I think

1. Computing md5 for a large number of files. This is only done once. should be removed and instead the (cleared away) md5sum bars should have a message Computing md5 for a large number of files
2. The Saving information to '*.dvc'. messages should be removed

[shcheklein]

@casperdcl @efiop if remove stuff (and bars after add is done) we need to show some message at least - one per file? Something like Adding foo1 ... ✔️. Or one message Added 10 files in 1234 seconds? It looks weird when we don't have anything at all at the (like for checkout/pull now).

[casperdcl]

new version with decreased verbosity:

Only thing missing is final stats

[casperdcl]

with some stats:

[casperdcl]

The recording clearly shows that md5sums only take about 10% of the total time. There must be other more time consuming functions.

[casperdcl]

@efiop think this is mergable as-is and then open a new issue/PR pair for refinement.

[efiop]

Looks great! Please see one minor naming comment.

[efiop]

Good start! Thank you!

[casperdcl]

@efiop not sure why you've started squash-merging PRs. Think history (esp for complex PRs like this one) is really important for tracking future issues...

[efiop]

@casperdcl This type of PR history would've been squashed anyway, because it is a total mess with "fix this" and "fix that" commits, which are a total nightmare to browse the history with. I usually prefer rebasing and squashing commits to make them nice and granular(and then do a regular merge of a nicely-formatted PR), but many contributors are used to just piling commits on top of each other(which is fine too), so I've decided to just squash everything to make it look nice in the history, while granular commits are available in the original PR on github along with all the context from the comments.

[casperdcl]

imho nothing beats git checkout feat && git rebase --interactive main && git checkout main && git merge --no-ff feat

[jorgeorpinel]

@casperdcl for commit message consistency please refer to https://dvc.org/doc/user-guide/contributing/core#commit-message-format-guidelines. Thanks

[casperdcl]

@jorgeorpinel sure; fine with most of it except don't agree with "Use dvc in a general case"

[jorgeorpinel]

Feel fee to submit a PR with a suggested update to that page and I'll tag the core dev team to discuss. 🙂