Windows path improvements

[MichaelAquilina]

@ranaur I reckon this PR should fix the windows path issues in a much simpler fashion :)

Could you test it out by installing as follows:

pip install git+https://github.com/MichaelAquilina/S4/@windows_path_improvements

(Fixes #156)

[codecov]

Codecov Report

Merging #171 into master will increase coverage by <.01%.
The diff coverage is 100%.

@@            Coverage Diff             @@
##           master     #171      +/-   ##
==========================================
+ Coverage   98.02%   98.02%   +<.01%     
==========================================
  Files          34       34              
  Lines        2328     2331       +3     
==========================================
+ Hits         2282     2285       +3     
  Misses         46       46

Impacted Files	Coverage Δ
tests/test_resolution.py	100% <ø> (ø)	⬆️
tests/test_sync.py	100% <ø> (ø)	⬆️
tests/commands/test_sync_command.py	100% <ø> (ø)	⬆️
s4/clients/s3.py	98.18% <100%> (+0.02%)	⬆️
tests/clients/test_s3.py	100% <100%> (ø)	⬆️
s4/clients/local.py	96.44% <100%> (+0.02%)	⬆️

---

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update e7b3e86...7cd814e. Read the comment docs.

[MichaelAquilina]

@ranaur please test this again by pulling the latest changes (which you provided in #175)

[ranaur]

Well ... I tested and I got the same bug I got with an earlier version. When syncing files in a subdirectory, it is syncing as "dir\file.ext" and not "dir/file.ext". So, in the s3 repository it show a file named "dir\file.ext" in the root dir, and not a folder named "dir", with a file names "file.ext" in it. I have a version that solves this problem, I'll commit it soon.

[ranaur]

In this version, the get_local_keys() method always returns files using "/" to separate paths, and the other methods expect the keys to be in this way.

[MichaelAquilina]

Hi @ranaur I would prefer to avoid complex abstractions like one you have proposed. I think the best action I can take right now is to add integration to AppVeyor so I can update code and test in a quicker fashion. I will get down to working on this as soon as I find the chance :)

[ranaur]

First I'll need to learn appveyor, but it is ok. About the code, have you take a look at it? The directory monitoring is quite different between the OSs, so the code should be very different, and "if os.name == 'nt'" is really ugly. The implementation wasn't a very high abstraction. I just took INotyfy_Recursive and implemented in another tree. What do you think is the best line of action for this?

…

On Sat, Dec 1, 2018 at 2:33 PM Michael Aquilina ***@***.***> wrote: Hi @ranaur <https://github.com/ranaur> I would prefer to avoid complex abstractions like one you have proposed. I think the best action I can take right now is to add integration to AppVeyor so I can update code and test in a quicker fashion. I will get down to working on this as soon as I find the chance :) — You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub <#171 (comment)>, or mute the thread <https://github.com/notifications/unsubscribe-auth/ACANM-iuWlZ36Us_KBuaP3YQU0umthZNks5u0q9jgaJpZM4YvDER> .

[MichaelAquilina]

My intention for Appveyor is to be able to provide support for windows by making sure that tests pass on Linux as well as Windows.

Without a CI to test, the project can not really support Windows so that is the first step.

Merging any more changes before that would be pre-mature as it would mean merging potentially unstable code upstream.

I have not taken a good look at the daemon code yet - but it does look like it needs to be simplified.

After I've got appveyor merged in (along with all the tests passing on Windows) - we can think about merging additional features (like looking into supporting a daemon on Windows).

[ranaur]

Hi, I took a small break on the project to handle "december matters". I noticed you dropped the use of "poetry". I tried to install the command using PIP. It works well, including a compiled (EXE) version of S4! However the master branch has the path handling problem fixed in "windows_path_improvements". Wasn't it merged yet? By the way I wisth you a Merry Chrismas and a happy new year!

…

On Mon, Dec 3, 2018 at 8:16 PM Michael Aquilina ***@***.***> wrote: My intention for Appveyor is to be able to provide support for windows by making sure that tests pass on Linux *as well as* Windows. Without a CI to test, the project can not really support Windows so that is the first step. Merging any more changes before that would be pre-mature as it would mean merging potentially unstable code upstream. I have not taken a good look at the daemon code yet - but it does look like it needs to be simplified. After I've got appveyor merged in (along with all the tests passing on Windows) - we can think about merging additional features (like looking into supporting a daemon on Windows). — You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub <#171 (comment)>, or mute the thread <https://github.com/notifications/unsubscribe-auth/ACANM-RACjNDWmafq2vkfecgPKnnYsZGks5u1aKlgaJpZM4YvDER> .