feat: server run command - #11
Closed
jason-lynch wants to merge 4 commits into
Closed
Conversation
This is a small change to serve the OpenAPI 3 specification instead of the OpenAPI 2 version. This change will support some automated API tooling, like restish.
This change makes the MQTT code more symmetrical with http.Server by deferring all errors until `Connect` is called. It makes this library a little more ergonomic because we only need to implement one set of error handling.
This commit adds a `run` command to the server module that starts an HTTP server and an HTTP-over-MQTT server. It also includes a configuration mechanism that supports configuration via a file, environment variables, or command line arguments. The HTTP server is enabled by default in this configuration. The MQTT server is disabled by default, but can optionally be enabled via a config file or environment variables.
jason-lynch
force-pushed
the
jason-lynch/server-run
branch
from
January 23, 2025 15:05
cb99aac to
c4ccaa5
Compare
Member
Author
|
The contents of this PR are included in #14. |
AntTheLimey
added a commit
that referenced
this pull request
Jul 21, 2026
The manifest pinned lakekeeper quay.io/lakekeeper/catalog:v0.9.0 — set in the foundational coldfront commit (f94b21d, Dave) and never refreshed. By that date v0.13.1 was already the latest, and there's no sign coldfront ever validated 0.9.0 (its compose floats on :latest, the native package defaults to 0.13.1). So the pin was stale, not a considered choice. Move to v0.13.1 — the current latest, the native-package default, and what coldfront's :latest resolves to today. Update the manifest block and the test fixtures/assertion that encode the version (service_images_test, the makeLakekeeperSpec fixtures, a service_spec_test image tag) plus the version-specific comment on catalogDBExtensions. Changelog review (v0.9.0 -> v0.13.1): no breaking changes on the surface CP uses — the /management/v1/{bootstrap,warehouse} endpoints, the /catalog/v1/{warehouse}/namespaces path, the migrate/serve/healthcheck subcommands, and the LAKEKEEPER__PG_* env vars are unchanged. Two watch-items for the end-to-end re-validation, neither client-breaking: v0.13.0 changed the default storage layout for new namespaces (// -> /), and switched migration locking to the built-in pg_advisory_xact_lock (so the catalog extension set is expected unchanged — re-confirm on the run). Enables the #11 follow-on: set LAKEKEEPER__DEBUG__MIGRATE_BEFORE_SERVE on serve, delete the migrate container, and drop the attachable overlay.
AntTheLimey
added a commit
that referenced
this pull request
Jul 21, 2026
Delete the standalone lakekeeper migrate one-shot container and let the serve container migrate the Iceberg catalog schema in-process on startup via LAKEKEEPER__DEBUG__MIGRATE_BEFORE_SERVE=true (finding #11 permanent form). This removes the only thing that needed the per-database overlay to be attachable, so that flag goes too. What changes: - service_spec.go: set LAKEKEEPER__DEBUG__MIGRATE_BEFORE_SERVE=true on the serve container. Give serve a longer health-check StartPeriod (2m vs the shared 30s) so the first-start in-process migration can't be marked unhealthy and restarted mid-migration; serve still goes healthy the instant its first check passes, and WaitForService's 5-minute budget still bounds a genuinely stuck serve. - Delete LakekeeperMigrateResource (file), its registration in resources.go, and its instantiation/wiring in orchestrator.go. - service_instance_spec.go: repoint the serve dependency. It previously depended on the migrate resource; now, in managed-catalog mode (catalog_db_create), it depends on LakekeeperCatalogDBResource so serve only starts once control-plane has created the catalog database. In external-catalog mode there is no such resource — the URL is validated at spec time — so no dependency is added (a dependency on a resource absent from the graph would be unsatisfiable). This mirrors the migrate resource's old conditional dependency. - network.go / orchestrator.go: drop the Attachable field and its NetworkCreate plumbing, and remove Attachable from the four <db>-database overlay literals. The field was this branch's own #11 stopgap and is not on main; ensureNetworks still validates user-supplied extra networks via the Docker inspect Attachable field, which is unrelated and untouched. Why serve-not-a-separate-migrate is safe: WaitForService returns on task state Running, and with a health check configured swarm gates Running on the check passing (the earlier #13 fix proved this — a broken health check made WaitForService time out). The check exercises serve's HTTP endpoint, so WaitForService returns only once serve is listening, i.e. after in-process migration. The bootstrap resource depends on the serve ServiceInstanceResource, so it still runs against a listening serve. Tests: assert MIGRATE_BEFORE_SERVE env and the longer StartPeriod on the serve spec; assert the serve->catalog-DB dependency in managed mode and its absence in external mode; drop the now-obsolete MigrateAttachesOverlay test. Ruled-out alternatives (bridge-IP migrate, apply-SQL-ourselves, temp hba hole, migrate-as-swarm-job) are recorded in the remediation report. make test green, gofmt + go vet clean. Multi-master caveat: with N serve replicas each would attempt the migration; safe single-node (idempotent/transactional), revisit a dedicated migrate job for multi-master (tracked in the readiness ledger).
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
1 participant
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.
This PR builds on top of #10 to add a
runcommand to the server module that uses the generated API specification. As of this PR, every API method intentionally returns a 500endpoint not implementederror. But it lays the groundwork for running and configuring the server.I touched a few modules to make this change, so I've split my changes up into one commit per module to make it easier to review. The name of the module is included in the commit header, e.g.
feat(api): ....