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 context parameter in services/repository #23186

Merged
merged 6 commits into from Feb 28, 2023

Conversation

wolfogre
Copy link
Member

Use context parameter in services/repository.

And use cache.WithCacheContext(ctx) to generate push action history feeds.

Fix #23160

@wolfogre wolfogre added the type/enhancement An improvement of existing functionality label Feb 28, 2023
@wolfogre wolfogre added this to the 1.20.0 milestone Feb 28, 2023
@GiteaBot GiteaBot added the lgtm/need 1 This PR needs approval from one additional maintainer to be merged. label Feb 28, 2023
@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 Feb 28, 2023
@lunny lunny added the reviewed/wait-merge This pull request is part of the merge queue. It will be merged soon. label Feb 28, 2023
@jolheiser
Copy link
Member

🎺 🤖

@jolheiser jolheiser merged commit 04347eb into go-gitea:main Feb 28, 2023
@jolheiser jolheiser removed the reviewed/wait-merge This pull request is part of the merge queue. It will be merged soon. label Feb 28, 2023
zjjhot added a commit to zjjhot/gitea that referenced this pull request Mar 1, 2023
* giteaofficial/main:
  [skip ci] Updated translations via Crowdin
  Properly flush unique queues on startup (go-gitea#23154)
  Use context parameter in services/repository (go-gitea#23186)
  Pass `--global` when calling `git config --get`, for consistency with `git config --set` (go-gitea#23157)
  Make `gitea serv` respect git binary home (go-gitea#23138)
  Write Gitpod `app.ini` only once (go-gitea#23192)
  Avoid too long names for actions (go-gitea#23162)
jolheiser pushed a commit that referenced this pull request Mar 8, 2023
Related to: #22294 #23186 #23054

Replace: #23218

Some discussion is in the comments of #23218.

Highlights:
- Add Expiration for cache context. If a cache context has been used for
more than 10s, the cache data will be ignored, and warning logs will be
printed.
- Add `discard` field to `cacheContext`, a `cacheContext` with `discard`
true will drop all cached data and won't store any new one.
- Introduce `WithNoCacheContext`, if one wants to run long-life tasks,
but the parent context is a cache context,
`WithNoCacheContext(perentCtx)` will discard the cache data, so it will
be safe to keep the context for a long time.
- It will be fine to treat an original context as a cache context, like
`GetContextData(context.Backgraud())`, no warning logs will be printed.

Some cases about nesting:

When:
- *A*, *B* or *C* means a cache context.
- ~*A*~, ~*B*~ or ~*C*~ means a discard cache context.
- `ctx` means `context.Backgrand()`
- *A(ctx)* means a cache context with `ctx` as the parent context.
- *B(A(ctx))* means a cache context with `A(ctx)` as the parent context.
- `With` means `WithCacheContext`
- `WithNo` means `WithNoCacheContext`

So:
- `With(ctx)` -> *A(ctx)*
- `With(With(ctx))` -> *A(ctx)*, not *B(A(ctx))*
- `With(With(With(ctx)))` -> *A(ctx)*, not *C(B(A(ctx)))*
- `WithNo(ctx)` -> *ctx*, not *~A~(ctx)*
- `WithNo(With(ctx))` -> *~A~(ctx)*
- `WithNo(WithNo(With(ctx)))` -> *~A~(ctx)*, not *~B~(~A~(ctx))*
- `With(WithNo(With(ctx)))` -> *B(~A~(ctx))*
- `WithNo(With(WithNo(With(ctx))))` -> *~B~(~A~(ctx))*
- `With(WithNo(With(WithNo(With(ctx)))))` -> *C(~B~(~A~(ctx)))*
@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/enhancement An improvement of existing functionality
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Too much warning "cannot get cache context when getting data"
5 participants