Resolve the fabriq brain by port, and split the e2e suite into its own module - #39
Merged
Merged
Conversation
EngineOption and EngineOptions named *fabriq.Fabriq to look the facade up in the container. buildToolkit already took an interface, so that one type parameter was the only thing dragging in fabriq's composition root, and with it ClickHouse, Elasticsearch, pgx, go-redis and Trove. The build graph was 1421 packages. Injecting core/query.Fabric takes it to 258 with every driver gone. An engine reached over the wire registers *remote.Fabric under the same key and nothing here changes. The registry now arrives as its own argument rather than off the facade. query.Fabric has no Registry() method on purpose: entity specs are declared client-side and a remote engine cannot serve them. Splitting it out created a failure the old code could not have, where the fabric resolves but the registry does not. That is fabriq wired wrong, not fabriq absent, and it is logged before the brain degrades to a no-op. An empty container stays quiet, since an app with no fabriq in it is not a misconfiguration. go.sum barely moves, because the integration test still opens a real embedded engine and a test-only import counts fully against the module graph. Splitting core out into its own module is what would fix that. The replace directive is local dev. Repin to a released fabriq and drop it before this merges.
The preceding commit swapped the concrete facade for the port interface but left go.mod untidied, so the tree would not build: go refused every command asking for a tidy first. This is that tidy. Testcontainers moves to v0.44.0 and stays direct, since the conformance suite drives real postgres and mongo containers. Build, vet and the full race suite pass, containers included.
The test opens a real embedded engine, which is the right thing for it to do and the reason cortex kept requiring root fabriq. go mod tidy counts a test import at full weight, so one file held the whole driver set in our go.sum. It lives in inttest/ now with its own go.mod. The main module requires fabriq/core and nothing else. The replaces are local development only.
The integration test in inttest/fabriqbrain_e2e_test.go lives in its own nested module, so nothing in this repo ran it: there was no test-integration target at all before this, unlike kgkit and fabriq. make test-integration now cds into inttest and runs it with the integration tag. CI here delegates build/test/lint to the shared xraph/workflows/.github/workflows/go-ci.yml reusable workflow, which the comment in ci.yml says probes for Makefile targets; that workflow lives in a separate repository outside this change, so whether it picks the new target up on its own could not be checked from here.
Rebasing onto main moved the dependency graph under this branch, so both go.mod files are what go mod tidy produces now rather than what it produced in June. The root module loses ninety-nine lines of it, which is the point of the split: testcontainers, elasticsearch and clickhouse belong to the integration suite rather than to anyone importing cortex. inttest also gained a replace for integrations/fabriq. That became its own module while this branch sat, so the root replace no longer reached it, and without one the suite would have been testing a published version of the integration instead of the tree it ships with.
registry.ServiceName is published now, so integrations/fabriq compiles against a real release rather than a working copy. fabriq also split core into its own module, so the parent goes up with it: holding the old parent alongside the new core made every core import ambiguous. The two replace directives pointing outside this repository are gone with it. They existed because ServiceName was unreleased, and while they were there the integration suite could only build on one machine. It vets under -tags integration on a clean checkout now, so CI can finally run it.
juicycleff
force-pushed
the
core-module-split
branch
from
September 3, 2026 13:40
db07102 to
819fdd2
Compare
juicycleff
marked this pull request as ready for review
September 3, 2026 13:40
Contributor
Author
|
Unblocked and out of draft. fabriq 1.6.4 publishes Both replace directives pointing outside this repository are gone with it. They only existed because Rebased onto current main, which includes the a2a work from #37.
|
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.
Draft, because it does not compile yet, and the reason is outside this repo. See the blocker at the bottom.
Two things, and the second is why the first was needed.
The fabriq brain resolves by port
injectFabricused to reach into the container for a concrete facade. It now asks forquery.Fabricand the entity registry by name, so a remote engine resolves exactly the way a local one does.The two ways that can fail are not the same failure and are no longer treated as one. No facade in the container means the app does not use fabriq, which is ordinary and stays quiet. A facade present but its registry missing means fabriq is wired wrong, so that one is logged before the brain degrades to a no-op. Losing a working brain silently is the outcome worth being noisy about.
The e2e suite moves into its own module
inttest/is a nested module now, and the root module is ninety-nine lines ofgo.modlighter for it. Testcontainers, elasticsearch and clickhouse belong to an integration suite, not to the dependency graph of everybody who imports cortex.make test-integrationruns it.What rebasing changed
This branch sat behind main by a hundred and three commits, so both
go.modfiles here are whatgo mod tidyproduces against current main rather than what it produced in June.inttestalso gained a replace forintegrations/fabriq. That became a module of its own while this branch sat, so the root replace no longer reached it, and without the new one the suite would have been exercising a published version of the integration rather than the tree it ships beside.The blocker
integrations/fabriqbuilds on main and does not build here:registry.ServiceNameexists in the working copy of fabriq at../../TwinOS/fabriqand in no published version ofgithub.com/xraph/fabriq, which is whatintegrations/fabriq/go.modrequires. So this needs a fabriq release carryingcore/registry.ServiceNamebefore it can go green, and the version bump is a one line change once there is one.Worth deciding separately:
inttest/go.modreplacesgithub.com/xraph/fabriqwith../../../../TwinOS/fabriq, a path outside this repository. That works on one machine and nowhere else, CI included. Fine for a suite gated behind a build tag and run by hand, and worth knowing about before anyone expects the target to work on their own checkout.What does pass
go build ./...andgo test ./...on the root module, with the usual mongo container failures that fail on main too.golangci-lintis clean. Theinttestmodule vets under-tags integration.Note on the other branch
di-inject-query-fabricheld the first two of these four commits and nothing else, so it is contained in this branch entirely. Nothing is lost by deleting it.