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

dvcfs: detach from pipeline outputs #7353

Merged
merged 1 commit into from
Feb 7, 2022
Merged

dvcfs: detach from pipeline outputs #7353

merged 1 commit into from
Feb 7, 2022

Conversation

efiop
Copy link
Contributor

@efiop efiop commented Feb 7, 2022

Introducing simple repo tree that combines all workspaces into one structure,
and allows us to detach data representation from pipelines.

Pre-requisite for dvc-data extraction and using relpaths in dvcfs/repofs.

Introducing simple repo tree that combines all workspaces into one structure,
and allows us to detach data representation from pipelines.

Pre-requisite for dvc-data extraction and using relpaths in dvcfs/repofs.
@efiop efiop requested a review from a team as a code owner February 7, 2022 10:20
@efiop efiop requested a review from karajan1001 February 7, 2022 10:20
@efiop efiop changed the title dvcfs: detach from pipeline outputs [WIP] dvcfs: detach from pipeline outputs Feb 7, 2022
@efiop efiop changed the title [WIP] dvcfs: detach from pipeline outputs dvcfs: detach from pipeline outputs Feb 7, 2022
Comment on lines +186 to +190
shortest = self.repo.index.tree.shortest_prefix(key)
if shortest:
assert shortest[1][1].isdir
if len(shortest[0]) <= len(key):
ret["isdvc"] = True
Copy link
Contributor Author

Choose a reason for hiding this comment

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

These odd flags and logic will be removed in a followup, when we just use fs and dvcfs separately (with subrepos), instead of passing through tons of odd flags used inappropriately in ls and diff.

@efiop efiop merged commit cb584c6 into iterative:main Feb 7, 2022
@efiop efiop added refactoring Factoring and re-factoring skip-changelog Skips changelog labels Feb 7, 2022
@@ -168,7 +168,7 @@ def _build_tree(fs_path, fs, name, **kwargs):
assert key
tree.add(key, meta, obj.hash_info)

tree_meta.size += meta.size
tree_meta.size += meta.size or 0
Copy link
Contributor

Choose a reason for hiding this comment

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

Excuse me, why can not just set them default 0? Like in C or Java current None is just like null point in other languages.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@karajan1001 This is a bit messy, but we have meta set to None for files inside of dvc add dir dirs. There seems to be a value in separating 0 and "not set" state, but you are right, we'll probably set it to defaults in the future.

if not odb:
continue

try:
Copy link
Contributor

Choose a reason for hiding this comment

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

Can reduce one line.

with suppress(FileNotFoundError, ObjectFormatError):

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
refactoring Factoring and re-factoring skip-changelog Skips changelog
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants