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

RSDK-9117 - Remove logger from NewModuleFromArgs, and moduleName from ModularMain #4482

Merged
merged 2 commits into from
Oct 25, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 1 addition & 3 deletions examples/customresources/demos/complexmodule/module.go
Original file line number Diff line number Diff line change
Expand Up @@ -15,10 +15,8 @@ import (
)

func main() {
// We will use a logging.Logger named "complexmodule," and our module will support 4 different
// resource models.
// ModularMain will stand up a module which will host 4 different models.
module.ModularMain(
"complexmodule",
Copy link
Member

Choose a reason for hiding this comment

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

sighhhhhhhhh

Copy link
Member

Choose a reason for hiding this comment

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

@cheukt I'm fine with leaving ModularMain alone. And just leaving a small comment about why it takes in a string that isn't used.

Copy link
Member

Choose a reason for hiding this comment

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

no it's okay, I'm being slightly melodramatic here. We will update the go modules eventually.

Will anything break the binaries currently uploaded to the registry? from my understanding, no.

Copy link
Member Author

@cheukt cheukt Oct 23, 2024

Choose a reason for hiding this comment

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

no, it will not break any current binaries, this will only break if you update rdk on the package

resource.APIModel{gizmoapi.API, mygizmo.Model},
resource.APIModel{summationapi.API, mysum.Model},
resource.APIModel{base.API, mybase.Model},
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,6 @@ import (
)

func main() {
// ModularMain will create a logger named "gizmomodule", and will stand up a module which will
// host our gizmo.
module.ModularMain("gizmomodule", resource.APIModel{gizmoapi.API, mygizmosummer.Model})
// ModularMain will stand up a module which will host our gizmo.
module.ModularMain(resource.APIModel{gizmoapi.API, mygizmosummer.Model})
}
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,6 @@ import (
)

func main() {
// ModularMain will create a logger named "summationmodule" and will then host a module with
// our mysum model in it.
module.ModularMain("summationmodule", resource.APIModel{summationapi.API, mysum.Model})
// ModularMain will host a module with our mysum model in it.
module.ModularMain(resource.APIModel{summationapi.API, mysum.Model})
}
Original file line number Diff line number Diff line change
Expand Up @@ -18,8 +18,7 @@ func main() {
camera.API,
model,
resource.Registration[camera.Camera, *fake.Config]{Constructor: newFakeCamera})

module.ModularMain("rtp-passthrough-camera", resource.APIModel{camera.API, model})
module.ModularMain(resource.APIModel{camera.API, model})
}

func newFakeCamera(
Expand Down
5 changes: 2 additions & 3 deletions examples/customresources/demos/simplemodule/module.go
Original file line number Diff line number Diff line change
Expand Up @@ -24,9 +24,8 @@ func main() {
Constructor: newCounter,
})

// Next, we run a module, which will log things as "simple-module." This module has a single
// model in it, the one we have created.
module.ModularMain("simple-module", resource.APIModel{generic.API, myModel})
// Next, we run a module which will have a singl model.
module.ModularMain(resource.APIModel{generic.API, myModel})
}

// newCounter is used to create a new instance of our specific model. It is called for each component in the robot's config with this model.
Expand Down
7 changes: 4 additions & 3 deletions module/module.go
Original file line number Diff line number Diff line change
Expand Up @@ -196,7 +196,8 @@ type Module struct {
robotpb.UnimplementedRobotServiceServer
}

// NewModule returns the basic module framework/structure.
// NewModule returns the basic module framework/structure. Use ModularMain and NewModuleFromArgs unless
// you really know what you're doing.
func NewModule(ctx context.Context, address string, logger logging.Logger) (*Module, error) {
// TODO(PRODUCT-343): session support likely means interceptors here
opMgr := operation.NewManager(logger)
Expand Down Expand Up @@ -263,11 +264,11 @@ func NewModule(ctx context.Context, address string, logger logging.Logger) (*Mod
}

// NewModuleFromArgs directly parses the command line argument to get its address.
func NewModuleFromArgs(ctx context.Context, logger logging.Logger) (*Module, error) {
func NewModuleFromArgs(ctx context.Context) (*Module, error) {
if len(os.Args) < 2 {
return nil, errors.New("need socket path as command line argument")
}
return NewModule(ctx, os.Args[1], logger)
return NewModule(ctx, os.Args[1], NewLoggerFromArgs(""))
}

// Start starts the module service and grpc server.
Expand Down
2 changes: 1 addition & 1 deletion module/multiversionmodule/module.go
Original file line number Diff line number Diff line change
Expand Up @@ -57,7 +57,7 @@ func main() {
}

func mainWithArgs(ctx context.Context, args []string, logger logging.Logger) error {
myMod, err := module.NewModuleFromArgs(ctx, logger)
myMod, err := module.NewModuleFromArgs(ctx)
if err != nil {
return err
}
Expand Down
6 changes: 3 additions & 3 deletions module/runtime.go
Original file line number Diff line number Diff line change
Expand Up @@ -11,9 +11,9 @@ import (

// ModularMain can be called as the main function from a module. It will start up a module with all
// the provided APIModels added to it.
func ModularMain(moduleName string, models ...resource.APIModel) {
func ModularMain(models ...resource.APIModel) {
mainWithArgs := func(ctx context.Context, args []string, logger logging.Logger) error {
mod, err := NewModuleFromArgs(ctx, logger)
mod, err := NewModuleFromArgs(ctx)
if err != nil {
return err
}
Expand All @@ -34,5 +34,5 @@ func ModularMain(moduleName string, models ...resource.APIModel) {
return nil
}

utils.ContextualMain(mainWithArgs, NewLoggerFromArgs(moduleName))
utils.ContextualMain(mainWithArgs, NewLoggerFromArgs(""))
}
2 changes: 1 addition & 1 deletion module/testmodule/main.go
Original file line number Diff line number Diff line change
Expand Up @@ -38,7 +38,7 @@ func mainWithArgs(ctx context.Context, args []string, logger logging.Logger) err
logger.Debug("debug mode enabled")

var err error
myMod, err = module.NewModuleFromArgs(ctx, logger)
myMod, err = module.NewModuleFromArgs(ctx)
if err != nil {
return err
}
Expand Down
2 changes: 1 addition & 1 deletion module/testmodule2/main.go
Original file line number Diff line number Diff line change
Expand Up @@ -35,7 +35,7 @@ func mainWithArgs(ctx context.Context, args []string, logger logging.Logger) err
logger.Debug("debug mode enabled")

var err error
myMod, err = module.NewModuleFromArgs(ctx, logger)
myMod, err = module.NewModuleFromArgs(ctx)
if err != nil {
return err
}
Expand Down
Loading