Skip to content

Remove dvcignore reset in repo._reset#4371

Merged
efiop merged 2 commits intotreeverse:masterfrom
karajan1001:dvcignore_optimize
Aug 11, 2020
Merged

Remove dvcignore reset in repo._reset#4371
efiop merged 2 commits intotreeverse:masterfrom
karajan1001:dvcignore_optimize

Conversation

@karajan1001
Copy link
Contributor

@karajan1001 karajan1001 commented Aug 10, 2020

fix #3869
Repo add,checkout,etc will not change .dvcignore, and can remove dvcignore reset after these operations.

Thank you for the contribution - we'll try to review it as soon as possible. 🙏

No performance improvement in dvc status(only influence commands using repo._reset())
Screenshot from 2020-08-10 22-47-39

@pared Excuse me, my dvc add tests failed.
On origin/master, clone the local repo to ./ then edit hashes.txt manually and run asv run HASHFILE:hashes.txt. And these is my error message.
Screenshot from 2020-08-10 22-54-53

Repo `add`,`checkout`,etc will not change `.dvcignore`, and can remove dvcignore reset after these operations.
@pared
Copy link
Contributor

pared commented Aug 10, 2020

@karajan1001
You need to pull the cats_dogs data from public repo, it should be configured inside repo. Please run:
dvc pull data/cats_dogs.dvc -r bench_s3_public

EDIT:
updated README: treeverse/dvc-bench#200

@pared pared requested a review from efiop August 10, 2020 15:19
@skshetry
Copy link
Collaborator

@karajan1001, one test is failing, need adjustment on:
https://github.com/iterative/dvc/blob/78458ee5a1f5d4423340494e0d25b24551e48748/tests/func/test_tree.py#L260

Just need to call after that location dvc_tree.__dict__.pop("dvcignore", None).

@codecov
Copy link

codecov bot commented Aug 10, 2020

Codecov Report

Merging #4371 into master will not change coverage.
The diff coverage is n/a.

Impacted file tree graph

@@           Coverage Diff           @@
##           master    #4371   +/-   ##
=======================================
  Coverage   90.92%   90.92%           
=======================================
  Files         178      178           
  Lines       12352    12352           
=======================================
  Hits        11231    11231           
  Misses       1121     1121           

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 78458ee...6cf5b93. Read the comment docs.

1. subrepo created during session open time.
Copy link
Collaborator

@skshetry skshetry left a comment

Choose a reason for hiding this comment

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

Feels like we need tree._reset() kind of API anyway for the tests. Though, looks good for now.

@efiop efiop merged commit 12c7013 into treeverse:master Aug 11, 2020
@karajan1001 karajan1001 deleted the dvcignore_optimize branch August 11, 2020 07:57
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.

optimize dvcignore

4 participants