-
Notifications
You must be signed in to change notification settings - Fork 1.2k
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
cloud versioning: imports #8789
Comments
Discussed a use case for this with @mnrozhkov. They have two repos:
They will try to use There is one aspect of |
This is estimated to take at least a sprint, so let's wait and see the demand for it. In the meantime, let's try to fail and say it's not supported. |
On the error handling, we probably just need to check if the remote is version_aware/worktree in repo dependency object collection: Line 131 in f68a125
|
@daavoo You are assigned here. Do you plan to add the exception that import is not supported for cloud versioning? |
@pmrowla I'm lost on how to fix the error handling here, but a user ran into this in discord. Would you be able to add the error handling 🙏 ? |
@efiop Do you want to take this one based on #9182 (comment), or is it premature? |
Yeah, taking this over. Support should land soon. |
If we are asked to open a potentially unitialized repo, we need to make sure it is not a bare git repo, so that we don't accidentally start putting dvc files inside git control dir. Discovered while working on iterative#8789
If we are asked to open a potentially unitialized repo, we need to make sure it is not a bare git repo, so that we don't accidentally start putting dvc files inside git control dir. Discovered while working on #8789
For now have to consolidate external_repo stuff and |
Happens when we've backed out our data to data storage but we are not sure if it is a directory or a file. Related iterative/dvc#8789
Happens when we've backed out our data to data storage but we are not sure if it is a directory or a file. Related iterative/dvc#8789
Happens when we've backed out our data to data storage but we are not sure if it is a directory or a file. Related iterative/dvc#8789
Happens when we've backed out our data to data storage but we are not sure if it is a directory or a file. Related iterative/dvc#8789
Makes dvc get/import/etc use dvcfs, which already supports cloud versioning, circular imports and other stuff. Also makes dvc import behave more like dvc import-url, so that we can use the same existing logic for fetching those using index instead of objects. Fixes iterative#8789 Related iterative/studio#4782
Makes dvc get/import/etc use dvcfs, which already supports cloud versioning, circular imports and other stuff. Also makes dvc import behave more like dvc import-url, so that we can use the same existing logic for fetching those using index instead of objects. Fixes iterative#8789 Related iterative/studio#4782
Makes dvc get/import/etc use dvcfs, which already supports cloud versioning, circular imports and other stuff. Also makes dvc import behave more like dvc import-url, so that we can use the same existing logic for fetching those using index instead of objects. Fixes #8789 Related iterative/studio#4782
I still owe a test for this though. Will add some in a moment... |
After taking another look, I don't think it is worth it, as it works through the same mechanisms that import/get/ls[-url] rely on. Will add explicit ones later on if there is need. |
Reopening per slack thread in https://iterativeai.slack.com/archives/CB41NAL8H/p1685768408126399. @efiop Do you recall if this was supposed to be closed and fully implemented? It looks that way, but I don't see any comments about it and can't remember the specifics. It doesn't seem to be working, and I got the same error trying a few different versions, so I'm not sure if it's a regression or it never worked. Here's what I get (it should be possible for anyone to reproduce from an empty repo if you have your credentials configured for the aws sandbox):
|
@dberenbaum Yeah, should be working. Taking a look right now... EDIT: can reproduce, taking a look... |
So indeed there is a bug in s3fs where we try to use |
@dberenbaum If you have time, feel free to give this a try:
Fixes for me locally and in tests. Will create a monkeypatch for dvc-s3 and a proper one for s3fs tomorrow. |
Works for me, thanks for the quick fix! |
For the record: fsspec/s3fs#746 |
Closing this issue since the last problem is just an s3fs bug and not really in the stuff we've implemented for cloud versioning and imports. |
import-url
imports I think it will not work right now unless the file has been pushed to a DVC remote, but when we get to refactoring regular cached objects to use dvc-data index for push/pull (so everything is unified to use the dvc-data index) this kind of chaining (from original source location) should work out of the box. (import-url: use dvc-dataindex.save()
for fetching imports #8249 (comment)_)The text was updated successfully, but these errors were encountered: