-
Notifications
You must be signed in to change notification settings - Fork 1.2k
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
Sort file by reverse lexicographical order before tailing them #2796
Conversation
Codecov Report
@@ Coverage Diff @@
## master #2796 +/- ##
==========================================
- Coverage 57.35% 57.34% -0.01%
==========================================
Files 393 393
Lines 25707 25721 +14
==========================================
+ Hits 14743 14749 +6
- Misses 9991 9999 +8
Partials 973 973
|
} | ||
// sort paths by descending filenames | ||
sort.SliceStable(paths, func(i, j int) bool { | ||
return filepath.Base(paths[i]) > filepath.Base(paths[j]) |
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.
Isn't that the same as switching i and j without having to reverse paths before ?
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.
No because here we sort only filename whereas the reverse ensure the whole filepath is reversed. This covers 2 distinct usecases:
├── abc123
│ ├── 1.log
│ └── 2.log
└── def456
├── 1.log
└── 2.log
where we want the 2.log
files to come out first and
├── 2018-01-17
│ └── error.log
└── 2018-01-18
└── error.log
There are some edge case that are not well handled though, mainly
├── 2018-01-17
│ ├── access.log
│ └── error.log # <- this one is tailed before
└── 2018-01-18
├── access.log # <- this one
└── error.log
or
├── abc123
│ ├── 1.log
│ ├── 2.log
│ ├── 3.log # <-┐
│ ├── 4.log # <-└ these are not active but tailed before
│ └── 5.log
└── def456
├── 1.log
└── 2.log # <- this one which is active in a k8s context
It's hard (impossible?) to find a good solution for this without that is also generic
968a52c
to
ee72698
Compare
What does this PR do?
Sort file by reverse lexicographiccal order before tailing them. The heuristic is that filenames often contain a datetime information
Motivation
What inspired you to submit this pull request?
Additional Notes
Anything else we should know when reviewing?