doctor: fixes unknown cache directory error on some windows mapped network shares #6364
Conversation
…work shares On Windows, resolve calls os.path._getfinalpathname, which calls GetFinalPathNameByHandle. This starts from the final resolved path of the file in the NT namespace (e.g. \Device\Mup\server\share...) and works backward as required to get the VOLUME_NAME_DOS. If the final path is a UNC share (i.e. \Device\Mup), We fix this by calling resolve() also on the partition mountpoints We also take the opportunity to make checks in case a PermissionError is returned to us Fixes github.com/treeverse/issues/6305
for more information, see https://pre-commit.ci
…work shares On Windows, resolve calls os.path._getfinalpathname, which calls GetFinalPathNameByHandle. This starts from the final resolved path of the file in the NT namespace (e.g. \Device\Mup\server\share...) and works backward as required to get the VOLUME_NAME_DOS. If the final path is a UNC share (i.e. \Device\Mup), We fix this by calling resolve() also on the partition mountpoints We also take the opportunity to make checks in case a PermissionError is returned to us Fixes treeverse#6305
|
The only problem is that hard to test it, and I can only get my NAS next weekend. |
| } | ||
| partition = {} | ||
| for part in psutil.disk_partitions(all=True): | ||
| if part.fstype != "": |
There was a problem hiding this comment.
Does this mean that psutil was not able to regognize fstype? If so, should we mark it as unknown then?
Could you elaborate on what part.mountpoint this happens on for you? On nas share or on something else?
There was a problem hiding this comment.
Yes, in some cases I had an empty fstype, I haven't managed to reproduce the error since but I'm sure it exists. That's why I added this check just in case. And yes you are right! It's better to mark it as "unknown" if that's the case, I can fix that if you want?
There was a problem hiding this comment.
I see. Yeah, if part.mountpoint is present and meaningful, let's adjust that to unknown, should make more sense.
There was a problem hiding this comment.
Hello ! Sorry for the delay... I looked again in the code and I think this case is already well managed, at dvc/info.py line 151, we're already returning "unknown" if we can't find what we want in our partition dict.
It looks good to me, what do you think ?
|
Sorry for late I will try it on my windows computer in this week end |
No problem, I will also be available this weekend if you need me. |
|
Current master
After this PR.
The workspace shows correctly. Thank you. |
3 participants
On Windows, resolve calls os.path._getfinalpathname, which calls GetFinalPathNameByHandle. This starts from the final resolved path of the file in the NT namespace (e.g. \Device\Mup\server\share...) and works backward as required to get the VOLUME_NAME_DOS. If the final path is a UNC share (i.e. \Device\Mup), it returns a path of the form ?\UNC\server\share.... pathlib translates this back to a regular UNC path of the form \server\share....
We fix this by calling resolve() also on the partition mountpoints and we also take the opportunity to make checks in case a PermissionError is returned to us
Fixes #6305
❗ I have followed the Contributing to DVC checklist.
📖 If this PR requires documentation updates, I have created a separate PR (or issue, at least) in dvc.org and linked it here.
Thank you for the contribution - we'll try to review it as soon as possible. 🙏