-
Notifications
You must be signed in to change notification settings - Fork 451
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
Centralize business logic for starting background prefetch. #184
Centralize business logic for starting background prefetch. #184
Conversation
f858bca
to
4d7a1d7
Compare
{ | ||
this.prefetchJobTimer = new Timer((state) => this.LaunchPrefetchJobIfIdle(), null, this.timerPeriod, this.timerPeriod); | ||
this.tracer.RelatedInfo(nameof(BackgroundPrefetcher) + ": starting background prefetch timer"); | ||
} | ||
else | ||
{ | ||
this.tracer.RelatedInfo(nameof(BackgroundPrefetcher) + ": no configured cache server, not starting background prefetch timer"); | ||
this.tracer.RelatedInfo(nameof(BackgroundPrefetcher) + ": no configured cache server or enlistment is unattended, not starting background prefetch timer"); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
For diagnosing issues, it would be nice to keep these as two separate trace statements, so we know which reason caused it to be disabled.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@sanoursa I considered that, but it added some complexity to this logging path for little value, because just a few lines prior to this statement in the mount process log file is "GVFS_UNATTENDED" if the enlistment is running in unattend mode, and at the head of the mount process log we log the remote url and objects endpoint url. I think that should give us the information we would need to diagnose.
Thoughts?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Got it, yea that makes sense
Move the check for unattended enlistment from InProcessMount to BackgroundPrefetcher.
4d7a1d7
to
553615d
Compare
Move the check for unattended enlistment from
InProcessMount
toBackgroundPrefetcher
.