Skip to content

Better sync detection and configuration via environmental variables #62

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 6 commits into from
Aug 15, 2017

Conversation

tekante
Copy link
Member

@tekante tekante commented Aug 15, 2017

This actually waits for the unison log file to stop growing before claiming that the initial sync is done.

@tekante tekante requested review from grayside and febbraro August 15, 2017 19:46
@grayside
Copy link
Contributor

Would mtime be simpler/more efficient?

@grayside
Copy link
Contributor

The failure of the branch implies the need to run go fmt on the touched files.

@tekante
Copy link
Member Author

tekante commented Aug 15, 2017

There doesn't appear to be any code advantage to trying to work with mtime, it's actually a little more complex due to needing to deal with the time type. It also doesn't appear to offer any speed advantages in terms of allowing a shorter pause between checks. It may be slightly more efficient but I don't have any data to know whether go is actually going to fetch all the stat data data regardless and then just push back the piece we need. Regardless, we're not checking frequently enough that I think efficiency at this scale is of concern.

I'll run stuff through gofmt and push up an update.

Side effect of increasing the timeout for container start but the actual syncing is the thing that has proven most problematic so far.
This prevents a spurious message about being unable to sync the tmp file showing in the logs as it was usually getting removed before the sync finished.
@febbraro febbraro merged commit 01df1dc into develop Aug 15, 2017
@febbraro febbraro deleted the feature/better-sync-detection branch August 15, 2017 22:37
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants