Skip to content

Conversation

@zeripath
Copy link
Contributor

Whilst debugging an issue on discord I noticed that we were logging:

2020/03/28 18:14:47 ...eue/queue_wrapped.go:161:Run() [F] Unable to set the internal queue for -wrapper Error: Unable to create queue level for  with cfg [123 ... 125] by max attempts: error: file missing [file=MANIFEST-000000]

because the cfg is possibly being passed as a []byte to log/fmt.Errorf. The solution is to check if the config is a []byte and if so cast it to a string before logging.

Signed-off-by: Andrew Thornton [email protected]

@zeripath zeripath added this to the 1.12.0 milestone Mar 28, 2020
@GiteaBot GiteaBot added the lgtm/need 1 This PR needs approval from one additional maintainer to be merged. label Mar 28, 2020
@codecov-io
Copy link

Codecov Report

Merging #10865 into master will increase coverage by 0.01%.
The diff coverage is 0.00%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master   #10865      +/-   ##
==========================================
+ Coverage   43.40%   43.41%   +0.01%     
==========================================
  Files         593      593              
  Lines       83271    83276       +5     
==========================================
+ Hits        36140    36154      +14     
+ Misses      42640    42636       -4     
+ Partials     4491     4486       -5     
Impacted Files Coverage Δ
modules/queue/queue_wrapped.go 6.28% <0.00%> (-0.19%) ⬇️
models/notification.go 63.88% <0.00%> (-0.84%) ⬇️
modules/queue/workerpool.go 58.00% <0.00%> (+1.06%) ⬆️
modules/git/repo.go 48.53% <0.00%> (+1.25%) ⬆️
modules/process/manager.go 78.31% <0.00%> (+3.61%) ⬆️
modules/indexer/stats/db.go 59.37% <0.00%> (+18.75%) ⬆️
modules/indexer/stats/queue.go 81.25% <0.00%> (+18.75%) ⬆️

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update ea67e56...ae06185. Read the comment docs.

@GiteaBot GiteaBot added lgtm/done This PR has enough approvals to get merged. There are no important open reservations anymore. and removed lgtm/need 1 This PR needs approval from one additional maintainer to be merged. labels Mar 29, 2020
@lunny
Copy link
Member

lunny commented Mar 29, 2020

make L-G-T-M

@lunny lunny merged commit e83daf7 into go-gitea:master Mar 29, 2020
@zeripath zeripath deleted the improve-logging-of-bad-cfg-in-queues branch March 29, 2020 07:41
@go-gitea go-gitea locked and limited conversation to collaborators Nov 24, 2020
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

lgtm/done This PR has enough approvals to get merged. There are no important open reservations anymore. type/bug

Projects

None yet

Development

Successfully merging this pull request may close these issues.

6 participants