Conversation
technoweenie
approved these changes
Nov 22, 2017
Contributor
technoweenie
left a comment
There was a problem hiding this comment.
What do you think of tasklog instead of tlog? I peeped the public API and agree it looks good:
type ListTask
func NewListTask(msg string) *ListTask
func (l *ListTask) Complete()
func (l *ListTask) Entry(update string)
func (l *ListTask) Throttled() bool
func (l *ListTask) Updates() <-chan *Update
type Logger
func NewLogger(sink io.Writer) *Logger
func (l *Logger) Close()
func (l *Logger) Enqueue(ts ...Task)
func (l *Logger) List(msg string) *ListTask
func (l *Logger) Percentage(msg string, total uint64) *PercentageTask
func (l *Logger) Waiter(msg string) *WaitingTask
type PercentageTask
func NewPercentageTask(msg string, total uint64) *PercentageTask
func (c *PercentageTask) Count(n uint64) (new uint64)
func (t *PercentageTask) Entry(update string)
func (c *PercentageTask) Throttled() bool
func (c *PercentageTask) Updates() <-chan *Update
type Task
type Update
func (u *Update) Throttled(next time.Time) bool
type WaitingTask
func NewWaitingTask(msg string) *WaitingTask
func (w *WaitingTask) Complete()
func (w *WaitingTask) Throttled() bool
func (w *WaitingTask) Updates() <-chan *Update
Contributor
Author
🤘 I like
🎉 Yay! |
chrisd8088
added a commit
to chrisd8088/git-lfs
that referenced
this pull request
May 26, 2023
In commit e90d54d of PR git-lfs#2329 the "git/githistory/log" package was added, including the PercentageTask structure and associated methods. Then in commit 122b765 of PR git-lfs#2610 the Entry() method was added to provide support for verbose logging in the "git lfs migrate impot" command. However, the name of the receiver argument for the Entry() method differs from that of the other methods for the PercentageTask structure, so we update it to match now, as well as fixing a typo which remains from the original commit in PR git-lfs#2329. Note that the "git/githistory/log" package was later renamed to the "tasklog" package in commits b9ab79e and 45c580e of PR git-lfs#2747.
chrisd8088
added a commit
to chrisd8088/git-lfs
that referenced
this pull request
May 26, 2023
In commit f100293 of PR git-lfs#2732 a local maxInt() function was added to the "git/githistory/log" package in order to avoid a circular dependancy on the "tools" package, which provides a common MaxInt() implementation. See, for reference, the discussion in this PR comment: git-lfs@f100293#r151551639 The "git/githistory/log" package was subsequently renamed to the "tasklog" package in commits b9ab79e and 45c580e of PR git-lfs#2747. At present, though, there is no cyclic dependency which prevents the use of the standard tools.MaxInt() function in the "tasklog" package, so we revert to its use now.
chrisd8088
added a commit
to chrisd8088/git-lfs
that referenced
this pull request
May 26, 2023
In commit e90d54d of PR git-lfs#2329 the "git/githistory/log" package was added, including the PercentageTask structure and associated methods, but unlike the WaitingTask which was added at the same time, no Complete() method was defined for the PercentageTask structure. (Note that the "git/githistory/log" package was later renamed to the "tasklog" package in commits b9ab79e and 45c580e of PR git-lfs#2747.) Other task types such as the ListTask and SimpleTask structures (introduced in commit 31ffeb9 of PR git-lfs#2335 and commit 7a760b6 of PR git-lfs#2756, respectively) provide a Complete() method, and the Meter task type of the "tq" package, which implemented the Task interface in commit 7c0f9e2 of PR git-lfs#2732, provides an equivalent Finish() method. These methods allow the caller to explicitly close the channel returned by the Updates() method, on which the anonymous goroutine started by the Logger.consume() method for the task is waiting, in a "range" loop on the channel in the Logger.logTask() method. One key use of the PercentageTask structure is in the methods of the Rewriter structure from the "git/githistory" package. It is initialized with the number of commits to be rewritten during a "git lfs migrate" command's traversal of a Git repository's history. As each commit is rewritten, the Count() method is called to increment the percentage completed value. When every commit has been rewritten, the count reaches the expected total, and the Count() method closes the channel, thus allowing the task receiving updates to finish and exit its goroutine. Under exceptional circumstances, though, the Rewrite() method of the Rewriter structure may never finish iterating through all the expected commits, and return in error or via a panic call. When this happens, the goroutine waiting on the PercentageTask's updates channel can never exit, leading to a hung process which never fully exits. In order to allow the Rewriter's Rewrite() method to define a deferred function which will always be called when it returns, under any circumstances, we add a Complete() method for the PercentageTask structure which sets the number of completed elements to the expected total in an atomic swap, and then closes the updates channel if the number of completed elements was previously less than the expected total. We also add a test to validate this new method's behaviour, and update the existing TestPercentageTaskCallsDoneWhenComplete() function to also confirm that calling the new Complete() method after the number of completed elements has reached the expected total does not cause a second attempt to close the updates channel.
chrisd8088
added a commit
to chrisd8088/git-lfs
that referenced
this pull request
May 26, 2023
In commit f100293 of PR git-lfs#2732 a local maxInt() function was added to the "git/githistory/log" package in order to avoid a circular dependancy on the "tools" package, which provides a common MaxInt() implementation. See, for reference, the discussion in this PR comment: git-lfs@f100293#r151551639 The "git/githistory/log" package was subsequently renamed to the "tasklog" package in commits b9ab79e and 45c580e of PR git-lfs#2747. At present, though, there is no cyclic dependency which prevents the use of the standard tools.MaxInt() function in the "tasklog" package, so we revert to its use now.
chrisd8088
added a commit
to chrisd8088/git-lfs
that referenced
this pull request
May 26, 2023
In commit e90d54d of PR git-lfs#2329 the "git/githistory/log" package was added, including the PercentageTask structure and associated methods, but unlike the WaitingTask which was added at the same time, no Complete() method was defined for the PercentageTask structure. (Note that the "git/githistory/log" package was later renamed to the "tasklog" package in commits b9ab79e and 45c580e of PR git-lfs#2747.) Other task types such as the ListTask and SimpleTask structures (introduced in commit 31ffeb9 of PR git-lfs#2335 and commit 7a760b6 of PR git-lfs#2756, respectively) provide a Complete() method, and the Meter task type of the "tq" package, which implemented the Task interface in commit 7c0f9e2 of PR git-lfs#2732, provides an equivalent Finish() method. These methods allow the caller to explicitly close the channel returned by the Updates() method, on which the anonymous goroutine started by the Logger.consume() method for the task is waiting, in a "range" loop on the channel in the Logger.logTask() method. One key use of the PercentageTask structure is in the methods of the Rewriter structure from the "git/githistory" package. It is initialized with the number of commits to be rewritten during a "git lfs migrate" command's traversal of a Git repository's history. As each commit is rewritten, the Count() method is called to increment the percentage completed value. When every commit has been rewritten, the count reaches the expected total, and the Count() method closes the channel, thus allowing the task receiving updates to finish and exit its goroutine. Under exceptional circumstances, though, the Rewrite() method of the Rewriter structure may never finish iterating through all the expected commits, and return in error or via a panic call. When this happens, the goroutine waiting on the PercentageTask's updates channel can never exit, leading to a hung process which never fully exits. In order to allow the Rewriter's Rewrite() method to define a deferred function which will always be called when it returns, under any circumstances, we add a Complete() method for the PercentageTask structure which sets the number of completed elements to the expected total in an atomic swap, and then closes the updates channel if the number of completed elements was previously less than the expected total. We also add a test to validate this new method's behaviour, and update the existing TestPercentageTaskCallsDoneWhenComplete() function to also confirm that calling the new Complete() method after the number of completed elements has reached the expected total does not cause a second attempt to close the updates channel.
chrisd8088
added a commit
to chrisd8088/git-lfs
that referenced
this pull request
Feb 25, 2025
In PR git-lfs#2329 the output of the "git lfs migrate" command was revised to use the methods of the Logger structure of the "log" package to output progress messages. This package was originally located within the "git/githistory" source directory, but was later moved and renamed to the "tlog" package and then to the current "tasklog" package in PR git-lfs#2747. As part of the changes in PR git-lfs#2329, the methods of the Rewriter structure in the "githistory" package were updated to use the methods of the Logger and PercentageTask types in the "log" package to generate progress messages which began with the Git LFS subcommand name, for example, "migrate: Rewriting commits". Since that PR, multiple other informational and error messages output by the "git lfs migrate" command have been added, and while most begin with the "migrate:" prefix, not all do. For instance, running the "git lfs migrate import --verbose" command outputs commit SHAs and file paths with the prefix, but Git references and the mapping of their old and new SHAs without the prefix. Two other Git LFS commands, the "git lfs prune" and "git lfs fetch" commands, follow the same design and output their subcommand names as prefixes to their progress messages, but no other commands do this. Git commands also do not prefix their progress messages with the name of the subcommand (although messages from a Git remote may be reported with the prefix "remote:"). To help bring all our commands' progress messages into alignment, we simply remove the "migrate:" prefix from the messages output by the "git lfs migrate" command. We have similarly altered the progress messages of the "git lfs prune" and "git lfs fetch" commands in previous commits in this PR.
chrisd8088
added a commit
to chrisd8088/git-lfs
that referenced
this pull request
Feb 25, 2025
In PR git-lfs#2329 the output of the "git lfs migrate" command was revised to use the methods of the Logger structure of the "log" package to output progress messages. This package was originally located within the "git/githistory" source directory, but was later moved and renamed to the "tlog" package and then to the current "tasklog" package in PR git-lfs#2747. As part of the changes in PR git-lfs#2329, the methods of the Rewriter structure in the "githistory" package were updated to use the methods of the Logger and PercentageTask types in the "log" package to generate progress messages which began with the Git LFS subcommand name, for example, "migrate: Rewriting commits". Since that PR, multiple other informational and error messages output by the "git lfs migrate" command have been added, and while most begin with the "migrate:" prefix, not all do. For instance, running the "git lfs migrate import --verbose" command outputs commit SHAs and file paths with the prefix, but Git references and the mapping of their old and new SHAs without the prefix. Two other Git LFS commands, the "git lfs prune" and "git lfs fetch" commands, follow the same design and output their subcommand names as prefixes to their progress messages, but no other commands do this. Git commands also do not prefix their progress messages with the name of the subcommand (although messages from a Git remote may be reported with the prefix "remote:"). To help bring all our commands' progress messages into alignment, we simply remove the "migrate:" prefix from the messages output by the "git lfs migrate" command. We have similarly altered the progress messages of the "git lfs prune" and "git lfs fetch" commands in previous commits in this PR.
chrisd8088
added a commit
to chrisd8088/git-lfs
that referenced
this pull request
Feb 25, 2025
In PR git-lfs#2329 the output of the "git lfs migrate" command was revised to use the methods of the Logger structure of the "log" package to output progress messages. This package was originally located within the "git/githistory" source directory, but was later moved and renamed to the "tlog" package and then to the current "tasklog" package in PR git-lfs#2747. As part of the changes in PR git-lfs#2329, the methods of the Rewriter structure in the "githistory" package were updated to use the methods of the Logger and PercentageTask types in the "log" package to generate progress messages which began with the Git LFS subcommand name, for example, "migrate: Rewriting commits". Since that PR, multiple other informational and error messages output by the "git lfs migrate" command have been added, and while most begin with the "migrate:" prefix, not all do. For instance, running the "git lfs migrate import --verbose" command outputs commit SHAs and file paths with the prefix, but Git references and the mapping of their old and new SHAs without the prefix. Two other Git LFS commands, the "git lfs prune" and "git lfs fetch" commands, follow the same design and output their subcommand names as prefixes to their progress messages, but no other commands do this. Git commands also do not prefix their progress messages with the name of the subcommand (although messages from a Git remote may be reported with the prefix "remote:"). To help bring all our commands' progress messages into alignment, we simply remove the "migrate:" prefix from the messages output by the "git lfs migrate" command. We have similarly altered the progress messages of the "git lfs prune" and "git lfs fetch" commands in previous commits in this PR.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
This pull request promotes the implementation of a task-based logging system found originally in
git/githistory/logto a new, top-level packagetlog.This accomplishes one of the goals that I proposed in #2743 (comment):
However, instead of calling it
log(confusing with Go's standard library packagelog) orgitlog(confusing withgit-log(1)-related utilities), chosen here istlogfor task-log. I think that this is a good name since it:Since this package contains a mostly-stable API, I feel comfortable promoting this package to live in the
github.com/git-lfs/git-lfs-namespace. Future goals in #2743 support adding new functionality to thelogpackage, which is something that I'd like to do after moving the code, and not before.Tracking: #2743
/cc @git-lfs/core #2743