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

Fix push/pull directory from DVC tracked tree #1781

Merged
merged 4 commits into from
May 31, 2022

Conversation

mattseddon
Copy link
Contributor

@mattseddon mattseddon commented May 27, 2022

2/3 main <- #1780 <- this <- #1783

Relates to iterative/dvc#7756.

This PR fixes out push/pull actions in the DVC tracked tree by collecting all tracked outs from the exp show data and feeding that information into the tree for the view.

Will not work as expected without some changes between DVC 2.10.2 and main so we are currently blocked.

@mattseddon mattseddon added the bug Something isn't working label May 27, 2022
@mattseddon mattseddon self-assigned this May 27, 2022
@mattseddon mattseddon changed the base branch from main to add-deps-outs-to-test-fixture May 27, 2022 05:42
@mattseddon mattseddon changed the base branch from add-deps-outs-to-test-fixture to main May 27, 2022 05:54
@mattseddon mattseddon changed the base branch from main to add-deps-outs-to-test-fixture May 27, 2022 05:55
Base automatically changed from add-deps-outs-to-test-fixture to main May 27, 2022 20:03
@mattseddon mattseddon force-pushed the improve-nested-pull-push branch 2 times, most recently from a20c055 to 1cf6754 Compare May 30, 2022 03:00
@mattseddon mattseddon marked this pull request as ready for review May 30, 2022 03:01
@@ -110,7 +127,7 @@ export class RepositoryModel
.map(([relativePath, status]) => {
const absolutePath = this.getAbsolutePath(relativePath)

if (!this.state.tracked.has(absolutePath)) {
Copy link
Contributor Author

Choose a reason for hiding this comment

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

[F] No longer can access this.state.tracked directly because it is made up of trackedLeafs and trackedNonLeafs. I have wrapped all access into some helper functions.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

We need to keep this.state.trackedLeafs separately so we can use it in collectTree.

@@ -1,8 +1,9 @@
import { dirname, resolve } from 'path'
import isEqual from 'lodash.isequal'
import { dirname, join, resolve } from 'path'
Copy link
Contributor Author

Choose a reason for hiding this comment

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

[F] Whole class (and probably more) will get rewritten when data:status lands.

@mattseddon mattseddon force-pushed the improve-nested-pull-push branch from 1cf6754 to 4f175c9 Compare May 31, 2022 00:04
@mattseddon mattseddon force-pushed the improve-nested-pull-push branch from 4f175c9 to b4268c6 Compare May 31, 2022 00:05
@codeclimate
Copy link

codeclimate bot commented May 31, 2022

Code Climate has analyzed commit b4268c6 and detected 0 issues on this pull request.

The test coverage on the diff in this pull request is 100.0% (85% is the threshold).

This pull request will bring the total coverage in the repository to 96.8% (0.0% change).

View more on Code Climate.

@mattseddon
Copy link
Contributor Author

mattseddon commented May 31, 2022

As stated in the description => Will not work as expected without some changes between DVC 2.10.2 and main so we are currently blocked.
but we can live with the behaviour until there is a DVC release, merging now.

@mattseddon mattseddon merged commit 39e0b95 into main May 31, 2022
@mattseddon mattseddon deleted the improve-nested-pull-push branch May 31, 2022 00:43
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants