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

codestyle: moves cache package to infra #17519

Merged
merged 1 commit into from Jun 13, 2019
Merged
Changes from all commits
Commits
File filter...
Filter file types
Jump to…
Jump to file or symbol
Failed to load files and symbols.

Always

Just for now

@@ -16,13 +16,13 @@ import (
httpstatic "github.com/grafana/grafana/pkg/api/static"
"github.com/grafana/grafana/pkg/bus"
"github.com/grafana/grafana/pkg/components/simplejson"
"github.com/grafana/grafana/pkg/infra/localcache"
"github.com/grafana/grafana/pkg/infra/log"
"github.com/grafana/grafana/pkg/infra/remotecache"
"github.com/grafana/grafana/pkg/middleware"
"github.com/grafana/grafana/pkg/models"
"github.com/grafana/grafana/pkg/plugins"
"github.com/grafana/grafana/pkg/registry"
"github.com/grafana/grafana/pkg/services/cache"
"github.com/grafana/grafana/pkg/services/datasources"
"github.com/grafana/grafana/pkg/services/hooks"
"github.com/grafana/grafana/pkg/services/quota"
@@ -60,7 +60,7 @@ type HTTPServer struct {
RenderService rendering.Service `inject:""`
Cfg *setting.Cfg `inject:""`
HooksService *hooks.HooksService `inject:""`
CacheService *cache.CacheService `inject:""`
CacheService *localcache.CacheService `inject:""`

This comment has been minimized.

Copy link
@torkelo

torkelo Jun 11, 2019

Member

why local cache? is the interface not a general one that can be backed by remote redis cache?

This comment has been minimized.

Copy link
@bergquist

bergquist Jun 11, 2019

Author Contributor

its not general. I think the usage pattern is different and should be explicit.
I renamed it to make it more clear that's it's local and in memory.

Choosing to cache stuff in memory or serialized remotely is two different actions IMO.

This comment has been minimized.

Copy link
@torkelo

torkelo Jun 11, 2019

Member

is it in memory, as so this is not the same cache that is backed by the database (that has lock issues?) Is this the in memory cache?

This comment has been minimized.

Copy link
@bergquist

bergquist Jun 11, 2019

Author Contributor

Oh, yea. Sorry for not being explicit about that.

This is the in-memory only cache. The cached backed by the database is named remotecache so naming this localcache felt more explicit.

This comment has been minimized.

Copy link
@bergquist

bergquist Jun 12, 2019

Author Contributor

@torkelo anything else to consider before we merge this?

This comment has been minimized.

Copy link
@marefr

marefr Jun 12, 2019

Member

@bergquist may affect enterprise datasource permissions maybe without having looked into the details of the changes

This comment has been minimized.

Copy link
@bergquist

bergquist Jun 12, 2019

Author Contributor

@marefr yes! Ive talked to Leo about it and I will open A pr fixing this in the enterprise so we can merge them at the same time.

DatasourceCache datasources.CacheService `inject:""`
AuthTokenService models.UserTokenService `inject:""`
QuotaService *quota.QuotaService `inject:""`
@@ -12,6 +12,7 @@ import (
"time"

"github.com/facebookgo/inject"
"github.com/patrickmn/go-cache"
"golang.org/x/sync/errgroup"

"github.com/grafana/grafana/pkg/api"
@@ -31,7 +32,6 @@ import (
"github.com/grafana/grafana/pkg/registry"
_ "github.com/grafana/grafana/pkg/services/alerting"
_ "github.com/grafana/grafana/pkg/services/auth"
"github.com/grafana/grafana/pkg/services/cache"
_ "github.com/grafana/grafana/pkg/services/cleanup"
_ "github.com/grafana/grafana/pkg/services/notifications"
_ "github.com/grafana/grafana/pkg/services/provisioning"
@@ -1,15 +1,17 @@
package cache
package localcache

import (
"time"

gocache "github.com/patrickmn/go-cache"
)

// CacheService cache any object in memory on the local instance.
type CacheService struct {
*gocache.Cache
}

// New returns a new CacheService
func New(defaultExpiration, cleanupInterval time.Duration) *CacheService {
return &CacheService{
Cache: gocache.New(defaultExpiration, cleanupInterval),
@@ -5,18 +5,18 @@ import (
"time"

"github.com/grafana/grafana/pkg/bus"
"github.com/grafana/grafana/pkg/infra/localcache"
m "github.com/grafana/grafana/pkg/models"
"github.com/grafana/grafana/pkg/registry"
"github.com/grafana/grafana/pkg/services/cache"
)

type CacheService interface {
GetDatasource(datasourceID int64, user *m.SignedInUser, skipCache bool) (*m.DataSource, error)
}

type CacheServiceImpl struct {
Bus bus.Bus `inject:""`
CacheService *cache.CacheService `inject:""`
Bus bus.Bus `inject:""`
CacheService *localcache.CacheService `inject:""`
}

func init() {
@@ -13,11 +13,11 @@ import (
"github.com/go-sql-driver/mysql"
"github.com/go-xorm/xorm"
"github.com/grafana/grafana/pkg/bus"
"github.com/grafana/grafana/pkg/infra/localcache"
"github.com/grafana/grafana/pkg/infra/log"
m "github.com/grafana/grafana/pkg/models"
"github.com/grafana/grafana/pkg/registry"
"github.com/grafana/grafana/pkg/services/annotations"
"github.com/grafana/grafana/pkg/services/cache"
"github.com/grafana/grafana/pkg/services/sqlstore/migrations"
"github.com/grafana/grafana/pkg/services/sqlstore/migrator"
"github.com/grafana/grafana/pkg/services/sqlstore/sqlutil"
@@ -50,9 +50,9 @@ func init() {
}

type SqlStore struct {
Cfg *setting.Cfg `inject:""`
Bus bus.Bus `inject:""`
CacheService *cache.CacheService `inject:""`
Cfg *setting.Cfg `inject:""`
Bus bus.Bus `inject:""`
CacheService *localcache.CacheService `inject:""`

dbCfg DatabaseConfig
engine *xorm.Engine
@@ -296,7 +296,7 @@ func InitTestDB(t ITestDB) *SqlStore {
sqlstore := &SqlStore{}
sqlstore.skipEnsureAdmin = true
sqlstore.Bus = bus.New()
sqlstore.CacheService = cache.New(5*time.Minute, 10*time.Minute)
sqlstore.CacheService = localcache.New(5*time.Minute, 10*time.Minute)

dbType := migrator.SQLITE

ProTip! Use n and p to navigate between commits in a pull request.
You can’t perform that action at this time.