Fix resyncing unnecessarily on windows machines. #865
Conversation
|
The bug was that windows path uses a back slash as opposed to a forward slash in the path. So, sorting on windows machines would differ from s3 order because of the back slashes. The fix was to replace all operating system separators with forward slashes before the sort and then add them back after the sort. @jamesls @danielgtaylor |
|
@kyleknap overall this looks like it will solve the issue, but I'm concerned about creating all the new copies of lists with names.sort(key=lambda item: item.replace(os.sep, '/'))Does that make sense? It will sort the list in-place and only temporarily create new strings as keys when comparing items. |
|
Oh, I did not know about that. It makes sense though. Definitely simplifies the code. I will probably remove |
|
I just made changes to simplify the code a bit. |
| @@ -231,6 +231,16 @@ def relative_path(filename, start=os.path.curdir): | |||
| return os.path.abspath(filename) | |||
|
|
|||
|
|
|||
| def normalize_sort(names, os_sep, character): | |||
There was a problem hiding this comment.
Is there any other code that would benefit from this being extracted out into this utils class? It seems very specific to the filegenerator class?
There was a problem hiding this comment.
No other classes or functions use it. Before the refactoring, there were two larger helper functions. Therefore, it made a little more sense at the time, but now it is just a single line of code. I will change it.
|
LGTM 🚢-it! |
|
|
Fix resyncing unnecessarily on windows machines.
Fixes issue: #843