Conversation
There was a problem hiding this comment.
Pull Request Overview
This PR adds multi-tenant support by introducing new modules, configuration sections, and services to handle tenant-specific settings. Key changes include adding the TenantAffixedEnvFeeder, new tenant configuration YAML files, and updates to various modules and the application framework to support tenant-aware operations.
Reviewed Changes
Copilot reviewed 36 out of 38 changed files in this pull request and generated 1 comment.
Show a summary per file
| File | Description |
|---|---|
| feeders/tenant_affixed_env.go | Introduces a new feeder with prefix/suffix functions for tenants |
| example_tenants/tenants/*.yaml | Adds tenant-specific configurations |
| example_tenants/modules.go | Implements tenant-aware modules and initializes tenant services |
| example_tenants/main.go | Updates main to register tenant-related services and modules |
| example/webserver/webserver.go | Adapts module signatures to use StdApplication |
| example/router/router.go | Updates router module to use StdApplication with minor cleanup |
| example/api/api.go | Adjusts API module initialization to adopt StdApplication types |
| config_provider*.go, application.go | Refactors logging and type signatures to support tenant-based configuration |
| README.md | Documents multi-tenancy support with examples |
| .github/workflows/ci.yml | Adds CI job for building and testing the multi-tenant example application |
Files not reviewed (2)
- example/go.mod: Language not supported
- example_tenants/go.mod: Language not supported
Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
There was a problem hiding this comment.
Pull Request Overview
This PR adds multi-tenancy support by introducing tenant-aware configuration and modules throughout the codebase. Key changes include the creation of a TenantAffixedEnvFeeder for environment variable transformation, new YAML tenant configuration examples, and updates to various modules (webserver, router, API, content, notifications) and the application interface to support tenant-specific operations.
Reviewed Changes
Copilot reviewed 36 out of 38 changed files in this pull request and generated no comments.
Show a summary per file
| File | Description |
|---|---|
| feeders/tenant_affixed_env.go | New feeder for tenant-specific environment variable prefix/suffix handling |
| example_tenants/tenants/*.yaml | New tenant configuration examples for different brands |
| example_tenants/modules.go | Added tenant-aware modules (ContentManager, NotificationManager) and updated initialization for multi-tenancy |
| example_tenants/main.go, example/api/api.go, example/router/router.go, etc. | Updated module registrations and changed Application usage to StdApplication |
| config_provider*.go, application.go | Refactored configuration and application lifecycle to support multi-tenancy, with changes to dependency injection and service registration |
| README.md | Enhanced documentation with multi-tenant support details and examples |
| .github/workflows/ci.yml | Updated CI to include building and testing the example tenants application |
Files not reviewed (2)
- example/go.mod: Language not supported
- example_tenants/go.mod: Language not supported
Comments suppressed due to low confidence (2)
example_tenants/modules.go:252
- [nitpick] Tenant configuration logging is triggered asynchronously via a goroutine in OnTenantRegistered. If log ordering or error handling is important, consider handling the result of cm.logTenantConfig synchronously or adding error capturing/documentation around the asynchronous behavior.
func (cm *ContentManager) OnTenantRegistered(tenantID modular.TenantID) {
example_tenants/modules.go:360
- [nitpick] Similar to the ContentManager, the NotificationManager triggers tenant config logging asynchronously using a goroutine. Consider reviewing whether asynchronous logging meets the design needs regarding log order and error propagation, or document the intended asynchronous behavior.
func (nm *NotificationManager) OnTenantRegistered(tenantID modular.TenantID) {
There was a problem hiding this comment.
Pull Request Overview
This PR adds multi-tenant support to the application framework while refactoring error handling and transitioning from the legacy Application type to a StdApplication interface. Key changes include:
- Introducing tenant-specific error constants and functions to load and retrieve tenant configurations.
- Refactoring module initialization, service registration, and configuration provider functions to integrate tenant-awareness.
- Updating tests, documentation, and CI workflows to reflect the new multi-tenant capabilities.
Reviewed Changes
Copilot reviewed 38 out of 40 changed files in this pull request and generated 1 comment.
Show a summary per file
| File | Description |
|---|---|
| feeders/affixed_env.go | Introduces new error constants for environment feeder validation. |
| example_tenants/*.yaml | Adds tenant-specific YAML configurations. |
| example_tenants/modules.go | Updates various modules (WebServer, Router, API, etc.) to support tenants. |
| example_tenants/main.go | Adjusts application creation and module registration for multi-tenancy. |
| config_provider.go & config_provider_test.go | Improves error wrapping and adjusts tests for configuration feeding. |
| application.go | Transitions to using StdApplication and integrates tenant service logic. |
| README.md | Documents multi-tenant support including tenant-aware modules and configs. |
| .github/workflows/ci.yml | Adds a CI job to build and test the example tenants application. |
Files not reviewed (2)
- example/go.mod: Language not supported
- example_tenants/go.mod: Language not supported
Comments suppressed due to low confidence (1)
config_provider.go:50
- While wrapping the main config feed error adds context, consider including the original error message explicitly to ease debugging in complex configurations.
return fmt.Errorf("config feed error: %w", err)
Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
…olangci/golangci-lint-action-8 Bump golangci/golangci-lint-action from 7 to 8
Added support for multi-tenant modules & configuration.