refactor(artifacts): use ArtifactEntry directly instead of subclassing #4649
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Codecov Report
Additional details and impacted files@@ Coverage Diff @@
## main #4649 +/- ##
==========================================
+ Coverage 82.61% 83.29% +0.67%
==========================================
Files 279 279
Lines 34927 34911 -16
==========================================
+ Hits 28854 29078 +224
+ Misses 6073 5833 -240
Flags with carried forward coverage won't be shown. Click here to find out more.
|
moredatarequired
added a commit
that referenced
this pull request
Dec 21, 2022
#4649) * Subsume ArtifactEntry into ArtifactManifestEntry
2 tasks
bcsherma
added a commit
that referenced
this pull request
Dec 29, 2022
commit d244d07 Author: Katia Patkin <87335417+kptkin@users.noreply.github.com> Date: Tue Dec 27 13:31:20 2022 -0800 style(public-api): format public file with proper formating (#4697) fix format commit 3dc8584 Author: Janosh Riebesell <janosh.riebesell@gmail.com> Date: Tue Dec 27 10:21:41 2022 -0800 feat(sdk): add `exist_ok=False` to `file.download()` (#4564) * add exist_ok=False to file.download() to avoid raising ValueError if file exists but you don't want to redownload also add type annotations * handle case file exists and exist_ok==True in File.download() * point out exist_ok=True option in file already exists error * tweak if/else paths Co-authored-by: Katia Patkin <87335417+kptkin@users.noreply.github.com> commit 4a4651e Author: Vish Rajiv <8609620+vwrj@users.noreply.github.com> Date: Fri Dec 23 14:27:16 2022 -0800 fix(artifacts): artifact.version should be the version index from the associated collection (#4486) fix(artifacts): artifact.version should be the version index from the associated collection Co-authored-by: Hugh Wimberly <hugh.wimberly@wandb.com> Co-authored-by: Dmitry Duev <dmitryduev@users.noreply.github.com> commit a7de372 Author: Noah Luna <15202580+ngrayluna@users.noreply.github.com> Date: Fri Dec 23 14:19:14 2022 -0800 docs(sdk): Removed less than, greater than characters from dosctrings… (#4687) docs(sdk): Removed less than, greater than characters from dosctrings. Re: It breaks the new doc engine Docusuarus. commit a45629d Author: Noah Luna <15202580+ngrayluna@users.noreply.github.com> Date: Fri Dec 23 14:18:00 2022 -0800 docs(sdk): Fixed typo in docstring for data_types.Objects3D (#4543) Fixed typo in docstring for data_types.Objects3D commit 6df4b88 Author: Dmitry Duev <dmitryduev@users.noreply.github.com> Date: Fri Dec 23 13:16:16 2022 -0800 test(integrations): fix import tests (#4690) test(integrations): fix import tests commit 8da62bb Author: Hugh Wimberly <hugh.wimberly@wandb.com> Date: Fri Dec 23 11:46:00 2022 -0800 refactor(artifacts): consolidate hash utilities into lib.hashutil (#4525) * Move all MD5 and base64 encode utilities to lib.hashutil * Remove duplicate function definitions * Refactor common code out into small functions * Use hypothesis to for testing * Remove unused code * Update type annotations for hash types commit eb70114 Author: Hugh Wimberly <hugh.wimberly@wandb.com> Date: Thu Dec 22 09:44:23 2022 -0800 test(artifacts): improve storage handler test coverage (#4674) * Add test case for adding a file to a finalized artifact * Add test case for caching local file references * Add GCS and WBArtifact storage handler tests commit 7106d17 Author: Hugh Wimberly <hugh.wimberly@wandb.com> Date: Wed Dec 21 19:36:31 2022 -0800 fix(artifacts): get digest directly instead of from the manifests' manifest (#4681) Reference digest directly instead of though the manifests' manifest commit 0a21fd1 Author: KyleGoyette <kdgoyette@gmail.com> Date: Wed Dec 21 14:37:03 2022 -0800 feat(launch): Default to using model-registry project for agent and launch_add (#4613) commit 84be24c Author: Griffin Tarpenning <griffin.tarpenning@wandb.com> Date: Wed Dec 21 13:20:02 2022 -0800 chore(launch): remove fallback resource when not specified for a queue (#4637) commit cf7df1d Author: speezepearson <speezepearson@users.noreply.github.com> Date: Tue Dec 20 17:10:13 2022 -0800 test(sdk): add tests for Api.upload_file_retry (#4639) commit c1ad0c0 Author: Hugh Wimberly <hugh.wimberly@wandb.com> Date: Tue Dec 20 16:08:37 2022 -0800 refactor(artifacts): use ArtifactEntry directly instead of subclassing (#4649) * Subsume ArtifactEntry into ArtifactManifestEntry commit 491fc59 Author: speezepearson <speezepearson@users.noreply.github.com> Date: Tue Dec 20 15:26:54 2022 -0800 test(sdk): add unit tests for filesync.StepUpload (#4652) * add unit tests for filesync.StepUpload commit 4e667d0 Author: Hugh Wimberly <hugh.wimberly@wandb.com> Date: Tue Dec 20 15:02:09 2022 -0800 fix(artifacts): correctly handle url-encoded local file references. (#4665) * Add test case for path names needing correct urldecoding * Use local_fil_uri_to_path instead of urlparse * Fix existing tests that used relative path uris instead of absolute paths
bcsherma
pushed a commit
that referenced
this pull request
Dec 29, 2022
#4649) * Subsume ArtifactEntry into ArtifactManifestEntry
bcsherma
added a commit
that referenced
this pull request
Dec 29, 2022
commit d244d07 Author: Katia Patkin <87335417+kptkin@users.noreply.github.com> Date: Tue Dec 27 13:31:20 2022 -0800 style(public-api): format public file with proper formating (#4697) fix format commit 3dc8584 Author: Janosh Riebesell <janosh.riebesell@gmail.com> Date: Tue Dec 27 10:21:41 2022 -0800 feat(sdk): add `exist_ok=False` to `file.download()` (#4564) * add exist_ok=False to file.download() to avoid raising ValueError if file exists but you don't want to redownload also add type annotations * handle case file exists and exist_ok==True in File.download() * point out exist_ok=True option in file already exists error * tweak if/else paths Co-authored-by: Katia Patkin <87335417+kptkin@users.noreply.github.com> commit 4a4651e Author: Vish Rajiv <8609620+vwrj@users.noreply.github.com> Date: Fri Dec 23 14:27:16 2022 -0800 fix(artifacts): artifact.version should be the version index from the associated collection (#4486) fix(artifacts): artifact.version should be the version index from the associated collection Co-authored-by: Hugh Wimberly <hugh.wimberly@wandb.com> Co-authored-by: Dmitry Duev <dmitryduev@users.noreply.github.com> commit a7de372 Author: Noah Luna <15202580+ngrayluna@users.noreply.github.com> Date: Fri Dec 23 14:19:14 2022 -0800 docs(sdk): Removed less than, greater than characters from dosctrings… (#4687) docs(sdk): Removed less than, greater than characters from dosctrings. Re: It breaks the new doc engine Docusuarus. commit a45629d Author: Noah Luna <15202580+ngrayluna@users.noreply.github.com> Date: Fri Dec 23 14:18:00 2022 -0800 docs(sdk): Fixed typo in docstring for data_types.Objects3D (#4543) Fixed typo in docstring for data_types.Objects3D commit 6df4b88 Author: Dmitry Duev <dmitryduev@users.noreply.github.com> Date: Fri Dec 23 13:16:16 2022 -0800 test(integrations): fix import tests (#4690) test(integrations): fix import tests commit 8da62bb Author: Hugh Wimberly <hugh.wimberly@wandb.com> Date: Fri Dec 23 11:46:00 2022 -0800 refactor(artifacts): consolidate hash utilities into lib.hashutil (#4525) * Move all MD5 and base64 encode utilities to lib.hashutil * Remove duplicate function definitions * Refactor common code out into small functions * Use hypothesis to for testing * Remove unused code * Update type annotations for hash types commit eb70114 Author: Hugh Wimberly <hugh.wimberly@wandb.com> Date: Thu Dec 22 09:44:23 2022 -0800 test(artifacts): improve storage handler test coverage (#4674) * Add test case for adding a file to a finalized artifact * Add test case for caching local file references * Add GCS and WBArtifact storage handler tests commit 7106d17 Author: Hugh Wimberly <hugh.wimberly@wandb.com> Date: Wed Dec 21 19:36:31 2022 -0800 fix(artifacts): get digest directly instead of from the manifests' manifest (#4681) Reference digest directly instead of though the manifests' manifest commit 0a21fd1 Author: KyleGoyette <kdgoyette@gmail.com> Date: Wed Dec 21 14:37:03 2022 -0800 feat(launch): Default to using model-registry project for agent and launch_add (#4613) commit 84be24c Author: Griffin Tarpenning <griffin.tarpenning@wandb.com> Date: Wed Dec 21 13:20:02 2022 -0800 chore(launch): remove fallback resource when not specified for a queue (#4637) commit cf7df1d Author: speezepearson <speezepearson@users.noreply.github.com> Date: Tue Dec 20 17:10:13 2022 -0800 test(sdk): add tests for Api.upload_file_retry (#4639) commit c1ad0c0 Author: Hugh Wimberly <hugh.wimberly@wandb.com> Date: Tue Dec 20 16:08:37 2022 -0800 refactor(artifacts): use ArtifactEntry directly instead of subclassing (#4649) * Subsume ArtifactEntry into ArtifactManifestEntry commit 491fc59 Author: speezepearson <speezepearson@users.noreply.github.com> Date: Tue Dec 20 15:26:54 2022 -0800 test(sdk): add unit tests for filesync.StepUpload (#4652) * add unit tests for filesync.StepUpload commit 4e667d0 Author: Hugh Wimberly <hugh.wimberly@wandb.com> Date: Tue Dec 20 15:02:09 2022 -0800 fix(artifacts): correctly handle url-encoded local file references. (#4665) * Add test case for path names needing correct urldecoding * Use local_fil_uri_to_path instead of urlparse * Fix existing tests that used relative path uris instead of absolute paths
bcsherma
added a commit
that referenced
this pull request
Jan 21, 2023
commit d244d07 Author: Katia Patkin <87335417+kptkin@users.noreply.github.com> Date: Tue Dec 27 13:31:20 2022 -0800 style(public-api): format public file with proper formating (#4697) fix format commit 3dc8584 Author: Janosh Riebesell <janosh.riebesell@gmail.com> Date: Tue Dec 27 10:21:41 2022 -0800 feat(sdk): add `exist_ok=False` to `file.download()` (#4564) * add exist_ok=False to file.download() to avoid raising ValueError if file exists but you don't want to redownload also add type annotations * handle case file exists and exist_ok==True in File.download() * point out exist_ok=True option in file already exists error * tweak if/else paths Co-authored-by: Katia Patkin <87335417+kptkin@users.noreply.github.com> commit 4a4651e Author: Vish Rajiv <8609620+vwrj@users.noreply.github.com> Date: Fri Dec 23 14:27:16 2022 -0800 fix(artifacts): artifact.version should be the version index from the associated collection (#4486) fix(artifacts): artifact.version should be the version index from the associated collection Co-authored-by: Hugh Wimberly <hugh.wimberly@wandb.com> Co-authored-by: Dmitry Duev <dmitryduev@users.noreply.github.com> commit a7de372 Author: Noah Luna <15202580+ngrayluna@users.noreply.github.com> Date: Fri Dec 23 14:19:14 2022 -0800 docs(sdk): Removed less than, greater than characters from dosctrings… (#4687) docs(sdk): Removed less than, greater than characters from dosctrings. Re: It breaks the new doc engine Docusuarus. commit a45629d Author: Noah Luna <15202580+ngrayluna@users.noreply.github.com> Date: Fri Dec 23 14:18:00 2022 -0800 docs(sdk): Fixed typo in docstring for data_types.Objects3D (#4543) Fixed typo in docstring for data_types.Objects3D commit 6df4b88 Author: Dmitry Duev <dmitryduev@users.noreply.github.com> Date: Fri Dec 23 13:16:16 2022 -0800 test(integrations): fix import tests (#4690) test(integrations): fix import tests commit 8da62bb Author: Hugh Wimberly <hugh.wimberly@wandb.com> Date: Fri Dec 23 11:46:00 2022 -0800 refactor(artifacts): consolidate hash utilities into lib.hashutil (#4525) * Move all MD5 and base64 encode utilities to lib.hashutil * Remove duplicate function definitions * Refactor common code out into small functions * Use hypothesis to for testing * Remove unused code * Update type annotations for hash types commit eb70114 Author: Hugh Wimberly <hugh.wimberly@wandb.com> Date: Thu Dec 22 09:44:23 2022 -0800 test(artifacts): improve storage handler test coverage (#4674) * Add test case for adding a file to a finalized artifact * Add test case for caching local file references * Add GCS and WBArtifact storage handler tests commit 7106d17 Author: Hugh Wimberly <hugh.wimberly@wandb.com> Date: Wed Dec 21 19:36:31 2022 -0800 fix(artifacts): get digest directly instead of from the manifests' manifest (#4681) Reference digest directly instead of though the manifests' manifest commit 0a21fd1 Author: KyleGoyette <kdgoyette@gmail.com> Date: Wed Dec 21 14:37:03 2022 -0800 feat(launch): Default to using model-registry project for agent and launch_add (#4613) commit 84be24c Author: Griffin Tarpenning <griffin.tarpenning@wandb.com> Date: Wed Dec 21 13:20:02 2022 -0800 chore(launch): remove fallback resource when not specified for a queue (#4637) commit cf7df1d Author: speezepearson <speezepearson@users.noreply.github.com> Date: Tue Dec 20 17:10:13 2022 -0800 test(sdk): add tests for Api.upload_file_retry (#4639) commit c1ad0c0 Author: Hugh Wimberly <hugh.wimberly@wandb.com> Date: Tue Dec 20 16:08:37 2022 -0800 refactor(artifacts): use ArtifactEntry directly instead of subclassing (#4649) * Subsume ArtifactEntry into ArtifactManifestEntry commit 491fc59 Author: speezepearson <speezepearson@users.noreply.github.com> Date: Tue Dec 20 15:26:54 2022 -0800 test(sdk): add unit tests for filesync.StepUpload (#4652) * add unit tests for filesync.StepUpload commit 4e667d0 Author: Hugh Wimberly <hugh.wimberly@wandb.com> Date: Tue Dec 20 15:02:09 2022 -0800 fix(artifacts): correctly handle url-encoded local file references. (#4665) * Add test case for path names needing correct urldecoding * Use local_fil_uri_to_path instead of urlparse * Fix existing tests that used relative path uris instead of absolute paths
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Description
Why do we have the
ArtifactEntry"interface" and theArtifactManifestEntry? I don't actually see that it buys us anything, and the extra indirection is a cost we'll pay every time we change things. If it's not worth it, let's just get rid of it.Testing
Just tested locally and in CI. Using the name that makes it into protos to avoid any larger changes.
Checklist