Merged
Conversation
|
Code coverage report:
WARNING: go tool cover failed for coverage_target.out |
0xAustinWang
approved these changes
Mar 17, 2026
Contributor
There was a problem hiding this comment.
Pull request overview
This PR consolidates the token verifier bootstrap/service wiring by removing the standalone servicefactory.go and inlining the tokenVerifierFactory implementation and helper coordinator builders into cmd/verifier/token/main.go.
Changes:
- Removed
cmd/verifier/token/servicefactory.go. - Moved the token verifier service factory (Start/Stop), coordinator creation, and source config building into
cmd/verifier/token/main.go.
Reviewed changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated 5 comments.
| File | Description |
|---|---|
| cmd/verifier/token/servicefactory.go | Deleted; its service factory/coordinator wiring is moved into main.go. |
| cmd/verifier/token/main.go | Now contains the token verifier service factory implementation and helper functions for coordinator/source config setup. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
You can also share your feedback on Copilot code review. Take the survey.
Comment on lines
+58
to
+75
| // Graceful shutdown | ||
| shutdownCtx, shutdownCancel := context.WithTimeout(context.Background(), 30*time.Second) | ||
| defer shutdownCancel() | ||
|
|
||
| var errs []error | ||
| if err := tvf.httpServer.Shutdown(shutdownCtx); err != nil { | ||
| tvf.lggr.Errorw("HTTP server shutdown error", "error", err) | ||
| errs = append(errs, fmt.Errorf("HTTP server shutdown error: %w", err)) | ||
| } | ||
|
|
||
| for _, coordinator := range tvf.coordinators { | ||
| if err := coordinator.Close(); err != nil { | ||
| tvf.lggr.Errorw("Coordinator shutdown error", "error", err) | ||
| errs = append(errs, fmt.Errorf("coordinator shutdown error: %w", err)) | ||
| } | ||
| } | ||
|
|
||
| tvf.lggr.Infow("Token verifier service stopped gracefully") |
Comment on lines
+129
to
+137
| db, err := cmd.ConnectToPostgresDB(tvf.lggr) | ||
| if err != nil { | ||
| tvf.lggr.Errorw("Failed to connect to Postgres database", "error", err) | ||
| os.Exit(1) | ||
| } | ||
|
|
||
| postgresStorage := storage.NewPostgres(db, tvf.lggr) | ||
| // Wrap storage with monitoring decorator to track query durations | ||
| monitoredStorage := storage.NewMonitoredStorage(postgresStorage, verifierMonitoring.Metrics()) |
Comment on lines
+105
to
+109
| _, err = cmd.StartPyroscope(tvf.lggr, config.PyroscopeURL, "tokenVerifier") | ||
| if err != nil { | ||
| tvf.lggr.Errorw("Failed to start pyroscope", "error", err) | ||
| os.Exit(1) | ||
| } |
Comment on lines
+81
to
+100
| func (tvf *tokenVerifierFactory) Start(ctx context.Context, appConfig token.ConfigWithBlockchainInfos, deps bootstrap.ServiceDeps) error { | ||
| logLevelStr := os.Getenv("LOG_LEVEL") | ||
| if logLevelStr == "" { | ||
| logLevelStr = "info" | ||
| } | ||
| var zapLevel zapcore.Level | ||
| if err := zapLevel.UnmarshalText([]byte(logLevelStr)); err != nil { | ||
| _, _ = fmt.Fprintf(os.Stderr, "Invalid LOG_LEVEL '%s', defaulting to 'info'\n", logLevelStr) | ||
| zapLevel = zapcore.InfoLevel | ||
| } | ||
| var err error | ||
| tvf.lggr, err = logger.NewWith(logging.DevelopmentConfig(zapLevel)) | ||
| if err != nil { | ||
| return fmt.Errorf("failed to create logger: %v", err) | ||
| } | ||
| tvf.lggr = logger.Named(tvf.lggr, "verifier") | ||
|
|
||
| // Use SugaredLogger for better API | ||
| tvf.lggr = logger.Sugared(tvf.lggr) | ||
|
|
Comment on lines
+48
to
+54
| type tokenVerifierFactory struct { | ||
| bootstrap.ServiceFactory[token.Config] | ||
|
|
||
| coordinators []*coordinator.Coordinator | ||
| httpServer *http.Server | ||
| lggr logger.Logger | ||
| } |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
There were some external dependencies which prevented #937 from working as expected. This should workaround the issue by putting all dependencies in a single file.