Skip to content

Conversation

@lunny
Copy link
Member

@lunny lunny commented Jan 26, 2021

Fix #14121, #14478.

The AccessLog middleware has to be after Contexter or APIContexter so that we can get LoginUserName if possible.
And also there is a BREAK change that it removed internal API access log.

@lunny lunny added type/bug skip-changelog This PR is irrelevant for the (next) changelog, for example bug fixes for unreleased features. labels Jan 26, 2021
@techknowlogick techknowlogick added this to the 1.14.0 milestone Jan 26, 2021
@GiteaBot GiteaBot added the lgtm/need 2 This PR needs two approvals by maintainers to be considered for merging. label Jan 26, 2021
@lunny lunny force-pushed the lunny/fix_access_log branch from 1498429 to dcbd2a4 Compare January 27, 2021 01:55
@lunny lunny force-pushed the lunny/fix_access_log branch from dcbd2a4 to 7429a01 Compare January 27, 2021 04:13
@lunny lunny added the pr/breaking Merging this PR means builds will break. Needs a description what exactly breaks, and how to fix it! label Jan 27, 2021
@6543
Copy link
Member

6543 commented Jan 27, 2021

@lunny can you use middleware instead of middlewares?

@lunny
Copy link
Member Author

lunny commented Jan 27, 2021

@lunny can you use middleware instead of middlewares?

see #14475 (comment)

@codecov-io
Copy link

Codecov Report

Merging #14475 (7429a01) into master (b2c20b6) will decrease coverage by 0.00%.
The diff coverage is 10.71%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master   #14475      +/-   ##
==========================================
- Coverage   42.11%   42.10%   -0.01%     
==========================================
  Files         758      760       +2     
  Lines       81085    81103      +18     
==========================================
+ Hits        34148    34149       +1     
- Misses      41360    41375      +15     
- Partials     5577     5579       +2     
Impacted Files Coverage Δ
modules/auth/sso/sso.go 26.92% <ø> (-5.22%) ⬇️
modules/context/access_log.go 0.00% <0.00%> (ø)
modules/context/context.go 59.13% <0.00%> (-2.51%) ⬇️
routers/api/v1/api.go 78.63% <0.00%> (-0.22%) ⬇️
routers/routes/base.go 24.03% <ø> (+5.38%) ⬆️
routers/routes/web.go 91.79% <0.00%> (ø)
modules/context/response.go 80.00% <25.00%> (-4.38%) ⬇️
modules/auth/sso/oauth2.go 47.61% <100.00%> (ø)
modules/middlewares/request.go 100.00% <100.00%> (ø)
modules/process/manager.go 72.50% <0.00%> (-2.50%) ⬇️
... and 3 more

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 b2c20b6...7429a01. Read the comment docs.

@GiteaBot GiteaBot added lgtm/need 1 This PR needs approval from one additional maintainer to be merged. and removed lgtm/need 2 This PR needs two approvals by maintainers to be considered for merging. labels Jan 27, 2021
@6543 6543 removed the skip-changelog This PR is irrelevant for the (next) changelog, for example bug fixes for unreleased features. label Jan 27, 2021
@6543
Copy link
Member

6543 commented Jan 27, 2021

☝️ since it's breaking ...

@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 Jan 27, 2021
@6543 6543 merged commit a51cc6d into go-gitea:master Jan 27, 2021
@lunny lunny deleted the lunny/fix_access_log branch January 28, 2021 01:31
@go-gitea go-gitea locked and limited conversation to collaborators Mar 11, 2021
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. pr/breaking Merging this PR means builds will break. Needs a description what exactly breaks, and how to fix it! type/bug

Projects

None yet

Development

Successfully merging this pull request may close these issues.

access.log stays empty

6 participants