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

Adjust transaction handling via db.Context #20031

Merged
merged 4 commits into from
Jun 20, 2022
Merged

Conversation

lunny
Copy link
Member

@lunny lunny commented Jun 19, 2022

Partially fix #19513

@lunny lunny added the type/refactoring Existing code has been cleaned up. There should be no new functionality. label Jun 19, 2022
models/db/context.go Outdated Show resolved Hide resolved
}
}

// InTransaction if context is in a transaction
func (ctx *Context) InTransaction() bool {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

What exactly is the purpose of this method?
It seems currently unused.
And also, when would that be good to know?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It's a requirement from #19513 , I think there are some requirements in some place but previously there is no suitable function to use.

@GiteaBot GiteaBot added the lgtm/need 2 This PR needs two approvals by maintainers to be considered for merging. label Jun 19, 2022
@Gusted Gusted added this to the 1.18.0 milestone Jun 19, 2022
@6543
Copy link
Member

6543 commented Jun 20, 2022

please resolve conflicts

@lunny
Copy link
Member Author

lunny commented Jun 20, 2022

please resolve conflicts

resolved.

Copy link
Member

@delvh delvh left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

So, to recap:
You opened a PR titled "Adjust something", and when asked where this something is needed, the answer boils down to "somewhere, perhaps".

No further questions. LGTM.

@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 Jun 20, 2022
@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 Jun 20, 2022
@6543 6543 changed the title Adjust something about db.Context Adjust transaction handling via db.Context Jun 20, 2022
@6543 6543 merged commit 0649c54 into go-gitea:main Jun 20, 2022
@lunny lunny deleted the lunny/db_context branch June 20, 2022 12:39
zjjhot added a commit to zjjhot/gitea that referenced this pull request Jun 21, 2022
* giteaofficial/main:
  [skip ci] Updated translations via Crowdin
  Catch the error before the response is processed by goth. (go-gitea#20000)
  Adjust transaction handling via db.Context (go-gitea#20031)
  Add more linters to improve code readability (go-gitea#19989)
  [skip ci] Updated translations via Crowdin
  Disable federation by default (go-gitea#20045)
  Respond with a 401 on git push when password isn't changed yet (go-gitea#20026)
  Alter hook_task TEXT fields to LONGTEXT (go-gitea#20038)
  Simplify and fix migration 216 (go-gitea#20035)
  use quoted regexp instead of git fixed-value (go-gitea#20029)
  fix delete pull head ref for DeleteIssue (go-gitea#20032)
  User keypairs and HTTP signatures for ActivityPub federation using go-ap (go-gitea#19133)
  Backtick table name in generic orphan check (go-gitea#20019)
  Update document to clarify that ALLOWED_DOMAINS/BLOCKED_DOMAINS support wildcard (go-gitea#20016)
  Return 404 when tag is broken (go-gitea#20017)
  Dump should only copy regular files and symlink regular files (go-gitea#20015)
vsysoev pushed a commit to IntegraSDL/gitea that referenced this pull request Aug 10, 2022
AbdulrhmnGhanem pushed a commit to kitspace/gitea that referenced this pull request Aug 24, 2022
@go-gitea go-gitea locked and limited conversation to collaborators May 3, 2023
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/refactoring Existing code has been cleaned up. There should be no new functionality.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Make sure if a DB Session got created for a request, it stays unique per request
5 participants