dynamicdan / sn-filesync

2-way sync ServiceNow field values to local files
MIT License
66 stars 37 forks source link

Resync issue on windows #8

Closed jakubpobiecky closed 8 years ago

jakubpobiecky commented 8 years ago

Hello dynamicdan,

I found an issue on windows running resync command(node.exe src\app --resync). I am still getting error: 'Failed to find root folder.' Issue is in src\file-record.js in method getRoot. I found cause of the problem: path.dirname(this.filePath) returns path with '/' and in config is '\' in filepath. I rewrote the method to: method.getRoot = function () { // cache if (this.rootDir) return this.rootDir;

var root = path.dirname(this.filePath).replace(/\//g, "\\");//issue
while (!this.config.roots[root]) {
    var up = path.dirname(root).replace(/\//g, "\\");//issue

    if (root === up) throw new Error('Failed to find root folder.');
    root = up;
}

return root;

}; I didn't want to make an commit, because I don't fully understand node.js and don't know what is affected in solution. Could you please test this fix on mac? On windows it looks like it works. Files are resynced.

And also for the future. It would be nice to have flag in config 'resyncOnStart' and it would only resynced files from server which are newer than local ones.(I think this check is already on conflict resolution)

Thanks, Jakub

dynamicdan commented 8 years ago

Thanks Jakub for reporting this. I also found this out recently and created a similar fix but it wasn't "strong" enough. There are other issues when handling the root path and searching for it in script. Now that I am forced to use windows for a customer, I will invest more time into this issue.

My workaround for now was to reset the file before I work on it (clear it and save to re-download).

Please let me know if any other path related issues popup with your fix. You could also try the --test option to run further tests (non-record-destructive tests).

The "resyncOnStart" config option currently exists as "_resyncFiles". It is not officially supported but you can use it for now (exactly the same as --resync).

Only syncing newer files is kind of a performance hit. I have to query for all the records anyway and have the new data. Adding a comparison of timestamps is then added complexity that is not needed IMO. In any case, I find that ~30 records takes < 3 seconds. So 100 records take less than 10 seconds and most setups don't have more... at least not yet. I am working on a search/auto-download record option so if --resync becomes a performance issue then I will re-visit it.

jakubpobiecky commented 8 years ago

Thanks for the quick answer :) Ok, if something will pop up I will let you know. For now, all files are resynced.

That's also an option to resync all files on load. This config option(_resyncFiles) is working for me. I don't mind a performance hit, because I am working on project with tens of people, so it is quite important for me, to have latest files.

And btw really nice work with the tool. Thanks, it helped me a lot

dynamicdan commented 8 years ago

I've just fixed this in the latest version 2.4.7. Note that your fix did not cover all scenarios but was close. I have now changed support for windows. You will need to update your config root path to my unix/universal style with "/" instead of "\" slashes.

jakubpobiecky commented 8 years ago

Thanks. I've tested that. It works.