The Wayback Machine - https://web.archive.org/web/20230113213308/https://github.com/go-gitea/gitea/pull/17027
Skip to content
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

Use common sessioner for API and Web #17027

Merged
merged 3 commits into from Sep 12, 2021

Conversation

zeripath
Copy link
Contributor

Instead of creating separate sessioner and doubly initialising the provider just
use the same sessioner for the API and Web routes.

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

Instead of creating separate sessioner and doubly initialising the provider just
use the same sessioner for the API and Web routes.

Signed-off-by: Andrew Thornton <[email protected]>
@zeripath zeripath added this to the 1.16.0 milestone Sep 12, 2021
@zeripath
Copy link
Contributor Author

If we merge in https://gitea.com/go-chi/session/pulls/1 then this will fix a data-race detected in https://drone.gitea.io/go-gitea/gitea/43900/2/14

routers/init.go Show resolved Hide resolved
6543
6543 approved these changes Sep 12, 2021
@codecov-commenter
Copy link

codecov-commenter commented Sep 12, 2021

Codecov Report

Merging #17027 (9905904) into main (aac7f68) will decrease coverage by 0.00%.
The diff coverage is 100.00%.

Impacted file tree graph

@@            Coverage Diff             @@
##             main   #17027      +/-   ##
==========================================
- Coverage   45.23%   45.22%   -0.01%     
==========================================
  Files         766      766              
  Lines       86641    86632       -9     
==========================================
- Hits        39191    39179      -12     
+ Misses      41108    41107       -1     
- Partials     6342     6346       +4     
Impacted Files Coverage Δ
routers/api/v1/api.go 78.19% <100.00%> (-0.29%) ⬇️
routers/init.go 62.96% <100.00%> (+5.82%) ⬆️
routers/web/web.go 90.40% <100.00%> (-0.13%) ⬇️
modules/git/repo_base_nogogit.go 82.85% <0.00%> (-2.86%) ⬇️
modules/git/utils.go 68.05% <0.00%> (-2.78%) ⬇️
models/issue_comment.go 51.17% <0.00%> (-0.59%) ⬇️
modules/queue/workerpool.go 49.23% <0.00%> (+1.14%) ⬆️
modules/queue/queue_channel.go 96.66% <0.00%> (+1.66%) ⬆️

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 aac7f68...9905904. Read the comment docs.

@6543 6543 merged commit a334a95 into go-gitea:main Sep 12, 2021
2 checks passed
@zeripath zeripath deleted the fix-race-in-virtual-session-provider branch Sep 12, 2021
@lunny
Copy link
Member

lunny commented Sep 13, 2021

APIs should be split as two parts, one is only for web, another is real API. The real API should remove Sessioner.

zeripath added a commit to zeripath/gitea that referenced this pull request Sep 13, 2021
Update to latest go-chi/session where the NewManager causes a new
Provider instantiation instead of reconfiguring an old one.
(https://gitea.com/go-chi/session/pulls/1)

The NewManager call is now concurrency safe and would allow live
reconfiguration in future but for now this PR simply fixes an
intermittent "data-race" detected in our tests.  (See
https://drone.gitea.io/go-gitea/gitea/43900/2/14)

Related  go-gitea#17027

Signed-off-by: Andrew Thornton <[email protected]>
zeripath added a commit to zeripath/gitea that referenced this pull request Sep 13, 2021
Update to latest go-chi/session where the NewManager causes a new
Provider instantiation instead of reconfiguring an old one.
(https://gitea.com/go-chi/session/pulls/1)

The NewManager call is now concurrency safe and would allow live
reconfiguration in future but for now this PR simply fixes an
intermittent "data-race" detected in our tests.  (See
https://drone.gitea.io/go-gitea/gitea/43900/2/14)

Related go-gitea#17027
Related go-gitea#1441

Signed-off-by: Andrew Thornton <[email protected]>
zeripath added a commit that referenced this pull request Sep 13, 2021
Update to latest go-chi/session where the NewManager causes a new
Provider instantiation instead of reconfiguring an old one.
(https://gitea.com/go-chi/session/pulls/1)

The NewManager call is now concurrency safe and would allow live
reconfiguration in future but for now this PR simply fixes an
intermittent "data-race" detected in our tests.  (See
https://drone.gitea.io/go-gitea/gitea/43900/2/14)

Related #17027
Related #1441

Signed-off-by: Andrew Thornton <[email protected]>
@go-gitea go-gitea locked and limited conversation to collaborators Oct 19, 2021
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Development

Successfully merging this pull request may close these issues.

None yet

6 participants