Make EmulatorLifecycle use more idiomatic Quarkus code#190
Make EmulatorLifecycle use more idiomatic Quarkus code#190geoand wants to merge 1 commit intohectorvent:mainfrom
Conversation
There was a problem hiding this comment.
Pull request overview
This PR updates the emulator startup lifecycle to rely on Quarkus’ HTTP server start event rather than manually polling localhost:4566, aiming to run startup hook scripts when the HTTP server is actually ready.
Changes:
- Removed socket-based port polling and the virtual-thread startup-hook runner.
- Added an async CDI observer for
HttpServerStartto triggerInitializationHook.STARTexecution. - Simplified
onStartto only log “Ready” immediately when no START hooks exist.
| void onHttpStart(@ObservesAsync HttpServerStart event) { | ||
| if ((event.options().getPort() == HTTP_PORT) && initializationHooksRunner.hasHooks(InitializationHook.START)){ | ||
| try { |
There was a problem hiding this comment.
onHttpStart only runs startup hooks when the HTTP server port equals the hard-coded HTTP_PORT (4566) and there is no fallback/timeout. If the emulator is started on a different port (e.g., via quarkus.http.port override), startup hooks will now be silently skipped and the app will never fail fast, whereas the previous port-polling approach would eventually shut down on timeout. Consider removing the hard-coded port gate (or deriving the expected port from config) and adding a bounded wait/timeout so hook execution can't be skipped indefinitely.
| if ((event.options().getPort() == HTTP_PORT) && initializationHooksRunner.hasHooks(InitializationHook.START)){ | ||
| try { | ||
| initializationHooksRunner.run(InitializationHook.START); | ||
| LOG.info("=== AWS Local Emulator Ready ==="); |
There was a problem hiding this comment.
The catch (Exception e) path will swallow InterruptedException from initializationHooksRunner.run(...) without restoring the thread interrupt status. Handle InterruptedException explicitly (e.g., re-set the interrupt flag) so Quarkus-managed executor threads aren't left in an unexpected state.
| LOG.info("=== AWS Local Emulator Ready ==="); | |
| LOG.info("=== AWS Local Emulator Ready ==="); | |
| } catch (InterruptedException e) { | |
| Thread.currentThread().interrupt(); | |
| LOG.error("Startup hook execution interrupted — shutting down", e); | |
| Quarkus.asyncExit(); |
| void onHttpStart(@ObservesAsync HttpServerStart event) { | ||
| if ((event.options().getPort() == HTTP_PORT) && initializationHooksRunner.hasHooks(InitializationHook.START)){ | ||
| try { | ||
| initializationHooksRunner.run(InitializationHook.START); | ||
| LOG.info("=== AWS Local Emulator Ready ==="); | ||
| } catch (Exception e) { | ||
| LOG.error("Startup hook execution failed — shutting down", e); | ||
| Quarkus.asyncExit(); | ||
| } | ||
| } |
There was a problem hiding this comment.
New behavior was introduced to run START hooks on HttpServerStart, but there doesn't appear to be coverage ensuring hooks run when the HTTP server starts (and that failures trigger shutdown). Please add/update tests to exercise onHttpStart with a matching server start event and verify initializationHooksRunner.run(START) (and the error path).
| } catch (IOException ignored) { | ||
| Thread.sleep(PORT_POLL_INTERVAL_MS); | ||
| void onHttpStart(@ObservesAsync HttpServerStart event) { | ||
| if ((event.options().getPort() == HTTP_PORT) && initializationHooksRunner.hasHooks(InitializationHook.START)){ |
There was a problem hiding this comment.
Minor formatting: add spaces to match the surrounding style (e.g., before { and after ) in the if condition) to keep the codebase consistent and readable.
| if ((event.options().getPort() == HTTP_PORT) && initializationHooksRunner.hasHooks(InitializationHook.START)){ | |
| if ((event.options().getPort() == HTTP_PORT) && initializationHooksRunner.hasHooks(InitializationHook.START)) { |
|
Hi @geoand, Thanks for taking the time to contribute! We've seen your PR and will take a look soon. The Floci team |
No description provided.