Copilot commented on code in PR #1340:
URL: https://github.com/apache/dubbo-admin/pull/1340#discussion_r2448661620
##########
pkg/core/runtime/runtime.go:
##########
@@ -115,13 +116,26 @@ func (rt *runtime) Add(components ...Component) {
func (rt *runtime) Start(stop <-chan struct{}) error {
components := maputil.Values(rt.components)
slice.SortBy(components, func(a, b Component) bool {
- return a.Order() < b.Order()
+ return a.Order() > b.Order()
})
for _, com := range components {
- err := com.Start(rt, stop)
- if err != nil {
- return err
- }
+ go func() {
+ err := com.Start(rt, stop)
+ if err != nil {
+ // if a core component failed to start, panic
+ if slice.Contain(CoreComponentTypes,
com.Type()) {
+ panic("core component " + com.Type() +
" running failed with error: " + err.Error())
+ } else {
+ logger.Errorf("component %s running
failed with error: %s", com.Type(), err.Error())
+ }
+ } else {
+ logger.Infof("component %s started
successfully", com.Type())
+ }
Review Comment:
Starting all components concurrently without waiting for initialization
dependencies could lead to race conditions. Components may depend on earlier
components being fully initialized (e.g., EventBus must be ready before
Engine). Consider adding synchronization or ordering guarantees for dependent
components.
##########
pkg/core/engine/component.go:
##########
@@ -69,40 +107,45 @@ func (b *engineComponent) Init(ctx runtime.BuilderContext)
error {
return err
}
for _, lw := range lwList {
- eventBusComponent, err :=
ctx.GetActivatedComponent(runtime.EventBus)
- if err != nil {
- return err
- }
- emitter, ok := eventBusComponent.(events.Emitter)
- if !ok {
- return fmt.Errorf("type assertion failed, event bus
component in runtime is not an Emitter")
- }
- storeComponent, err :=
ctx.GetActivatedComponent(runtime.ResourceStore)
- if err != nil {
- return err
- }
- resourceStore, ok := storeComponent.(store.ResourceStore)
- if !ok {
- return fmt.Errorf("type assertion failed, resource
store component in runtime is not a ResourceStore")
- }
rk := lw.ResourceKind()
- newFunc, err :=
coremodel.ResourceSchemaRegistry().NewResourceFunc(rk)
+ rs, err := e.storeRouter.ResourceKindRoute(rk)
if err != nil {
- return err
+ return fmt.Errorf("can not find store for resource kind
%s, %w", rk, err)
+ }
+ informer := controller.NewInformerWithOptions(lw, emitter, rs,
controller.Options{ResyncPeriod: 0})
+ if lw.TransformFunc() != nil {
+ err = informer.SetTransform(lw.TransformFunc())
+ if err != nil {
+ return fmt.Errorf("can not set transform for
informer of resource kind %s, %w", rk, err)
Review Comment:
Corrected 'can not' to 'cannot' for consistency.
##########
pkg/core/engine/component.go:
##########
@@ -41,25 +45,59 @@ type Component interface {
var _ Component = &engineComponent{}
type engineComponent struct {
- name string
- informers []controller.Informer
+ name string
+ storeRouter store.Router
+ informers []controller.Informer
+ subscriptionManager events.SubscriptionManager
+ subscribers []events.Subscriber
}
func newEngineComponent() Component {
return &engineComponent{
- informers: make([]controller.Informer, 0),
+ informers: make([]controller.Informer, 0),
+ subscribers: make([]events.Subscriber, 0),
}
}
-func (b *engineComponent) Type() runtime.ComponentType {
+func (e *engineComponent) Type() runtime.ComponentType {
return runtime.ResourceEngine
}
-func (b *engineComponent) Order() int {
- return math.MaxInt
+func (e *engineComponent) Order() int {
+ return math.MaxInt - 3
}
-func (b *engineComponent) Init(ctx runtime.BuilderContext) error {
+func (e *engineComponent) Init(ctx runtime.BuilderContext) error {
cfg := ctx.Config().Engine
+ e.name = cfg.Name
+ eventBusComponent, err := ctx.GetActivatedComponent(runtime.EventBus)
+ if err != nil {
+ return fmt.Errorf("can not retrieve event bus from runtime in
engine %s, %w", cfg.Name, err)
+ }
+ eventBus, ok := eventBusComponent.(events.EventBus)
+ if !ok {
+ return bizerror.NewAssertionError("EventBus",
reflect.TypeOf(eventBusComponent).Name())
+ }
+ e.subscriptionManager = eventBus
+ storeComponent, err := ctx.GetActivatedComponent(runtime.ResourceStore)
+ if err != nil {
+ return fmt.Errorf("can not retrieve store from runtime in
engine %s, %w", e.name, err)
Review Comment:
Corrected 'can not' to 'cannot' for consistency.
```suggestion
return fmt.Errorf("cannot retrieve store from runtime in engine
%s, %w", e.name, err)
```
##########
pkg/core/engine/component.go:
##########
@@ -69,40 +107,45 @@ func (b *engineComponent) Init(ctx runtime.BuilderContext)
error {
return err
}
for _, lw := range lwList {
- eventBusComponent, err :=
ctx.GetActivatedComponent(runtime.EventBus)
- if err != nil {
- return err
- }
- emitter, ok := eventBusComponent.(events.Emitter)
- if !ok {
- return fmt.Errorf("type assertion failed, event bus
component in runtime is not an Emitter")
- }
- storeComponent, err :=
ctx.GetActivatedComponent(runtime.ResourceStore)
- if err != nil {
- return err
- }
- resourceStore, ok := storeComponent.(store.ResourceStore)
- if !ok {
- return fmt.Errorf("type assertion failed, resource
store component in runtime is not a ResourceStore")
- }
rk := lw.ResourceKind()
- newFunc, err :=
coremodel.ResourceSchemaRegistry().NewResourceFunc(rk)
+ rs, err := e.storeRouter.ResourceKindRoute(rk)
if err != nil {
- return err
+ return fmt.Errorf("can not find store for resource kind
%s, %w", rk, err)
+ }
+ informer := controller.NewInformerWithOptions(lw, emitter, rs,
controller.Options{ResyncPeriod: 0})
+ if lw.TransformFunc() != nil {
+ err = informer.SetTransform(lw.TransformFunc())
+ if err != nil {
+ return fmt.Errorf("can not set transform for
informer of resource kind %s, %w", rk, err)
+ }
}
- informer := controller.NewInformerWithOptions(lw, emitter,
resourceStore,
- newFunc(), controller.Options{ResyncPeriod: 0})
- b.informers = append(b.informers, informer)
+ e.informers = append(e.informers, informer)
+ logger.Infof("resource engine %s has added informer for
resource kind %s", e.name, rk)
+ }
+ return nil
+}
+
+func (e *engineComponent) initSubscribers(eventbus events.EventBus) error {
+ rs, err := e.storeRouter.ResourceKindRoute(meshresource.InstanceKind)
+ if err != nil {
+ return fmt.Errorf("can not find store for resource kind %s,
%w", meshresource.RuntimeInstanceKind, err)
Review Comment:
Corrected 'can not' to 'cannot' for consistency.
```suggestion
return fmt.Errorf("cannot find store for resource kind %s, %w",
meshresource.RuntimeInstanceKind, err)
```
##########
pkg/core/engine/component.go:
##########
@@ -41,25 +45,59 @@ type Component interface {
var _ Component = &engineComponent{}
type engineComponent struct {
- name string
- informers []controller.Informer
+ name string
+ storeRouter store.Router
+ informers []controller.Informer
+ subscriptionManager events.SubscriptionManager
+ subscribers []events.Subscriber
}
func newEngineComponent() Component {
return &engineComponent{
- informers: make([]controller.Informer, 0),
+ informers: make([]controller.Informer, 0),
+ subscribers: make([]events.Subscriber, 0),
}
}
-func (b *engineComponent) Type() runtime.ComponentType {
+func (e *engineComponent) Type() runtime.ComponentType {
return runtime.ResourceEngine
}
-func (b *engineComponent) Order() int {
- return math.MaxInt
+func (e *engineComponent) Order() int {
+ return math.MaxInt - 3
}
-func (b *engineComponent) Init(ctx runtime.BuilderContext) error {
+func (e *engineComponent) Init(ctx runtime.BuilderContext) error {
cfg := ctx.Config().Engine
+ e.name = cfg.Name
+ eventBusComponent, err := ctx.GetActivatedComponent(runtime.EventBus)
+ if err != nil {
+ return fmt.Errorf("can not retrieve event bus from runtime in
engine %s, %w", cfg.Name, err)
Review Comment:
Corrected 'can not' to 'cannot' for consistency.
##########
pkg/core/discovery/component.go:
##########
@@ -110,21 +113,26 @@ func (d *discoveryComponent) newInformers(cfg
*discovery.Config, ctx runtime.Bui
if err != nil {
return nil, err
}
- emitter := eventBusComponent.(events.Emitter)
+ emitter, ok := eventBusComponent.(events.Emitter)
+ if !ok {
+ return nil, bizerror.NewAssertionError("Emitter",
reflect.TypeOf(eventBusComponent).Name())
+ }
storeComponent, err := ctx.GetActivatedComponent(runtime.ResourceStore)
if err != nil {
- return nil, err
+ return nil, fmt.Errorf("can not retrieve store from runtime in
engine %s, %w", cfg.Name, err)
Review Comment:
Corrected 'can not' to 'cannot' for consistency.
```suggestion
return nil, fmt.Errorf("cannot retrieve store from runtime in
engine %s, %w", cfg.Name, err)
```
##########
pkg/core/engine/component.go:
##########
@@ -69,40 +107,45 @@ func (b *engineComponent) Init(ctx runtime.BuilderContext)
error {
return err
}
for _, lw := range lwList {
- eventBusComponent, err :=
ctx.GetActivatedComponent(runtime.EventBus)
- if err != nil {
- return err
- }
- emitter, ok := eventBusComponent.(events.Emitter)
- if !ok {
- return fmt.Errorf("type assertion failed, event bus
component in runtime is not an Emitter")
- }
- storeComponent, err :=
ctx.GetActivatedComponent(runtime.ResourceStore)
- if err != nil {
- return err
- }
- resourceStore, ok := storeComponent.(store.ResourceStore)
- if !ok {
- return fmt.Errorf("type assertion failed, resource
store component in runtime is not a ResourceStore")
- }
rk := lw.ResourceKind()
- newFunc, err :=
coremodel.ResourceSchemaRegistry().NewResourceFunc(rk)
+ rs, err := e.storeRouter.ResourceKindRoute(rk)
if err != nil {
- return err
+ return fmt.Errorf("can not find store for resource kind
%s, %w", rk, err)
Review Comment:
Corrected 'can not' to 'cannot' for consistency.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
