Skip to content
This repository has been archived by the owner on Sep 13, 2023. It is now read-only.

art sizes and hashes #131

Merged
merged 1 commit into from
Dec 10, 2021
Merged

art sizes and hashes #131

merged 1 commit into from
Dec 10, 2021

Conversation

mike0sv
Copy link
Contributor

@mike0sv mike0sv commented Dec 10, 2021

closes #90

@mike0sv mike0sv requested a review from aguschin December 10, 2021 11:54
@mike0sv mike0sv self-assigned this Dec 10, 2021
@mike0sv mike0sv requested a review from a team December 10, 2021 11:54
@codecov
Copy link

codecov bot commented Dec 10, 2021

Codecov Report

Merging #131 (bfcf0a6) into main (9100d10) will increase coverage by 0.06%.
The diff coverage is 92.47%.

Impacted file tree graph

@@            Coverage Diff             @@
##             main     #131      +/-   ##
==========================================
+ Coverage   80.43%   80.49%   +0.06%     
==========================================
  Files          57       59       +2     
  Lines        3542     3835     +293     
==========================================
+ Hits         2849     3087     +238     
- Misses        693      748      +55     
Impacted Files Coverage Δ
mlem/cli/utils.py 60.41% <0.00%> (-0.41%) ⬇️
mlem/pack/docker/context.py 35.11% <0.00%> (ø)
mlem/contrib/dvc.py 37.68% <11.11%> (-1.72%) ⬇️
mlem/polydantic/lazy.py 85.29% <44.44%> (-14.71%) ⬇️
mlem/api/commands.py 75.89% <66.66%> (ø)
mlem/core/objects.py 90.64% <96.00%> (-0.18%) ⬇️
mlem/contrib/callable.py 97.72% <97.72%> (ø)
mlem/cli/info.py 97.29% <100.00%> (ø)
mlem/config.py 97.91% <100.00%> (-0.05%) ⬇️
mlem/contrib/catboost.py 96.96% <100.00%> (+0.04%) ⬆️
... and 18 more

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 2103fa0...bfcf0a6. Read the comment docs.

@mike0sv
Copy link
Contributor Author

mike0sv commented Dec 10, 2021

iterative/dvc#3069

Copy link
Contributor

@aguschin aguschin left a comment

Choose a reason for hiding this comment

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

Looks good 👍
Would be good if you add few words here about why you went with md5 and not some other, considering that issue you posted ^

@mike0sv
Copy link
Contributor Author

mike0sv commented Dec 10, 2021

I went with it because I did it before I read all that =)
For now we use hashes only to change metafile when artifact changes (we don't even validate when downloading stuff from remotes). For this purpose, speed is the preference.
We can add validation and switch algo if we want, but I see that as dvc scope, and we can rely on it in the future

@mike0sv mike0sv merged commit 0070e31 into main Dec 10, 2021
@mike0sv mike0sv deleted the feature/artifact-meta branch December 10, 2021 18:30
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Add hashes and size of artifacts to meta
2 participants