Fix/period in folder #27
Conversation
| """Get s3 key.""" | ||
| ret_key = "/".join([str(s.name) for s in self.key_segments]) | ||
| # We'll infer folder/file based on extension | ||
| return ret_key if '.' in self.key_segments[-1].name else f"{ret_key}/" | ||
| # HACK to catch case where the "extension" is actually a part of the folder name | ||
| # (eg, a folder named "v0.1.0"), we check if the extension is actually a number | ||
| name = self.key_segments[-1].name | ||
| ext = name.split('.')[-1] | ||
| return ret_key if (ext is not name and not ext.isdigit()) else f"{ret_key}/" |
There was a problem hiding this comment.
Could we just check the is_file property on the last key segment, or is it possible that it's inconsistent?
Technically S3 objects don't need extensions at all, so checking for "." seems problematic (which is my own fault).
There was a problem hiding this comment.
yeah as far as i can tell the is_file property has to be explicitly set currently (it isn't set when initing a MirrorPath)
| except Exception as e: | ||
| # 404 occurs if the key is a "folder" or does not exist | ||
| if "404" in str(e): | ||
| return False |
There was a problem hiding this comment.
Is there a specific exception that could be caught?
Unlikely case but a different error with a key that contains "404" could still hit this, maybe.
There was a problem hiding this comment.
probably yeah, i initially tried using the botocore ClientError exception but for some reason it wasn't catching properly
There was a problem hiding this comment.
I'll just tag #23 and let it slow-cook on the backburner
is_file_on_s3util to usehead_objectwhich seems to be more consistent. This does change behavior of that function to not raise an exception if the key is not found, since the 404 error can also just mean that the key is not a valid file (read: its a folder). Instead, users should use a combination ofexists_on_s3andis_file_on_s3to properly handle all cases.s3_utilsinstead ofmirror_path