Conversation
| } | ||
| go func() { | ||
| lggr.Infow("🌐 HTTP API server starting", "port", "8100") | ||
| if err := httpServer.ListenAndServe(); err != nil && err != http.ErrServerClosed { | ||
| lggr.Errorw("HTTP server error", "error", err) | ||
| tvf.lggr.Infow("🌐 HTTP API server starting", "port", "8100") | ||
| if err := tvf.httpServer.ListenAndServe(); err != nil && !errors.Is(err, http.ErrServerClosed) { | ||
| // TODO: how to register an error with the bootstrapper? | ||
| tvf.lggr.Errorw("HTTP server error", "error", err) | ||
| } | ||
| }() | ||
|
|
||
| lggr.Infow("🎯 Verifier service fully started and ready!") | ||
|
|
||
| // Wait for shutdown signal | ||
| <-sigCh | ||
| lggr.Infow("🛑 Shutdown signal received, stopping verifier...") | ||
|
|
||
| // Graceful shutdown | ||
| shutdownCtx, shutdownCancel := context.WithTimeout(context.Background(), 30*time.Second) | ||
| defer shutdownCancel() | ||
|
|
||
| if err := httpServer.Shutdown(shutdownCtx); err != nil { |
There was a problem hiding this comment.
This part moved to the Stop function.
winder
force-pushed
the
will/bootstrap-token-verifier
branch
from
f6667ad to
c20db88
Compare
| if b.jdConfig != nil { | ||
| return b.startWithJDLifecycle(ctx) | ||
| } | ||
| if b.appCfg != nil { | ||
| return b.startWithAppConfig(ctx) | ||
| } |
There was a problem hiding this comment.
The new With options essentially toggle whether the JD lifecycle object is used or if the start/stop functions are called directly.
| func (b *Bootstrapper[AppConfig]) connectToDB(ctx context.Context) (*sqlx.DB, error) { | ||
| db, err := sqlx.ConnectContext(ctx, "postgres", b.cfg.DB.URL) | ||
| func connectToDB(ctx context.Context, connStr string) (*sqlx.DB, error) { | ||
| db, err := sqlx.ConnectContext(ctx, "postgres", connStr) |
There was a problem hiding this comment.
Pure functions simplify testing.
| configPath := os.Getenv(ConfigPathEnv) | ||
| if configPath == "" { | ||
| configPath = DefaultConfigPath | ||
| } | ||
|
|
||
| cfg, err := LoadConfig(configPath) | ||
| if err != nil { | ||
| return fmt.Errorf("failed to load config: %w", err) | ||
| } |
There was a problem hiding this comment.
Moved into the constructor so that the config path can be controlled via Options.
| err := bootstrap.Run( | ||
| "TokenVerifier", | ||
| &tokenVerifierFactory{}, | ||
| // TODO: remove the AppConfig generic type to streamline this API, update factory to accept config as a string. | ||
| bootstrap.WithTOMLAppConfig[token.ConfigWithBlockchainInfos]("/etc/config.toml"), |
There was a problem hiding this comment.
New bootstrap function configured to use the config as AppConfig rather than JD Lifecycle Config.
There was a problem hiding this comment.
Copy/pasted from the original main() function and split into Start/Stop functions.
| db, err := b.connectToDB(ctx) | ||
| // startWithAppConfig is a passthrough to the application's Start function. | ||
| func (b *Bootstrapper[AppConfig]) startWithAppConfig(ctx context.Context) error { | ||
| return b.fac.Start(ctx, *b.appCfg, ServiceDeps{}) |
There was a problem hiding this comment.
I wonder if we'd still wanna pass in the keystore in this app config passthru mode... I guess I'm a fan of being consistent where possible
The token verifier doesn't need it right now but future apps might
There was a problem hiding this comment.
Maybe its as simple as calling connectToDB and initializeKeystore above
There was a problem hiding this comment.
Agree. In my first attempt I had several options like WithJD(), WithDB(), WithKeystore(), etc. I think it makes a lot of sense to treat these as different components the app can request from the bootstrap layer.
There was a problem hiding this comment.
We need to have a discussion about what interface we want to expose from the bootstrapper.
I'm starting to think it should launch the app being bootstrapped in a separate process rather than being compiled in.
| registry := accessors.NewRegistry(blockchainHelper) | ||
| cmd.RegisterEVM(ctx, registry, tvf.lggr, blockchainHelper, config.OnRampAddresses, config.RMNRemoteAddresses) |
There was a problem hiding this comment.
To make this chain-agnostic I think we need something similar to what we have for commit, where the accessor factory is created and returns the chain-specific impls of the source readers
There was a problem hiding this comment.
Can do in a follow up though
There was a problem hiding this comment.
Makes sense, this PR was focusing on installing the bootstrap layer. Extracting the factory registration makes sense as a followup.
winder
force-pushed
the
will/bootstrap-token-verifier
branch
from
818496f to
6200bec
Compare
|
Code coverage report:
|
winder
changed the title
3 participants
bootstrapinterface forTokenVerifierby splitting the existing token verifier main.go entrypoint into separateStartandStopfunctions.bootstrapto support running with direct configurations as an alternative to JD.