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

import-url: include files entry for cloud versioned dir dependencies #8528

Merged
merged 5 commits into from
Nov 4, 2022

Conversation

pmrowla
Copy link
Contributor

@pmrowla pmrowla commented Nov 4, 2022

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

Will fix #8506

@pmrowla pmrowla added A: data-sync Related to dvc get/fetch/import/pull/push A: cloud-versioning Related to cloud-versioned remotes labels Nov 4, 2022
@pmrowla pmrowla self-assigned this Nov 4, 2022
Comment on lines +382 to +385
if not self.hash_info and self.hash_name != "md5":
md5 = getattr(self.meta, "md5", None)
if md5:
self.hash_info = HashInfo("md5", md5)
Copy link
Contributor Author

Choose a reason for hiding this comment

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

for directories our hash_info is always a dvc md5 (even though the filesystem based output.hash_name may not be md5)

if not dep.obj:
dep.obj = dep.get_obj()
entry.obj = dep.obj
return entry
Copy link
Contributor Author

Choose a reason for hiding this comment

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

still undecided if this function belongs in output.get_entry or if we need something like repo.index.build() that takes an output or stage and generates index entries

@codecov
Copy link

codecov bot commented Nov 4, 2022

Codecov Report

Base: 94.28% // Head: 94.25% // Decreases project coverage by -0.03% ⚠️

Coverage data is based on head (c45b410) compared to base (d7152e3).
Patch coverage: 74.50% of modified lines in pull request are covered.

Additional details and impacted files
@@            Coverage Diff             @@
##             main    #8528      +/-   ##
==========================================
- Coverage   94.28%   94.25%   -0.04%     
==========================================
  Files         431      431              
  Lines       32812    32825      +13     
  Branches     4576     4582       +6     
==========================================
+ Hits        30937    30938       +1     
- Misses       1459     1467       +8     
- Partials      416      420       +4     
Impacted Files Coverage Δ
dvc/dependency/base.py 60.00% <50.00%> (-0.98%) ⬇️
dvc/output.py 90.42% <73.17%> (-0.95%) ⬇️
dvc/repo/index.py 93.54% <100.00%> (+0.85%) ⬆️
tests/unit/repo/experiments/conftest.py 88.67% <0.00%> (-5.67%) ⬇️
dvc/repo/experiments/queue/utils.py 76.66% <0.00%> (-3.34%) ⬇️
dvc/repo/experiments/queue/celery.py 84.68% <0.00%> (-0.48%) ⬇️

Help us with your feedback. Take ten seconds to tell us how you rate us. Have a feature suggestion? Share it here.

☔ View full report at Codecov.
📢 Do you have feedback about the report comment? Let us know in this issue.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A: cloud-versioning Related to cloud-versioned remotes A: data-sync Related to dvc get/fetch/import/pull/push
Projects
None yet
Development

Successfully merging this pull request may close these issues.

import-url: --version-aware does not apply to directories
1 participant