docs(publishing): make the Publisher guides match the shipped server#331
Open
mlennie wants to merge 11 commits into
Open
docs(publishing): make the Publisher guides match the shipped server#331mlennie wants to merge 11 commits into
mlennie wants to merge 11 commits into
Conversation
Four fixes, all verified against the published @malloy-publisher/server@0.0.228.
Environment variable substitution: the page said Publisher does not support it
and told readers to put real credential values in the config file. Both halves
are wrong. config.ts substitutes any ${VAR} in a string value, including package
locations, so credentials can stay in the environment. Documented the actual
matching rule (uppercase, braced only) and the failure mode when a variable is
missing: the config silently fails to load and the server comes up serving
nothing while /api/v0/status still reports "serving".
Reserved connection name: the attached-database examples declared an
environment-level connection named "duckdb", which is reserved for the
per-package sandbox. Booting that config logs "Error initializing environment
...; skipping environment" and drops the entire environment, so the server
reports "serving" with zero environments. Renamed to shared_duckdb, updated the
Malloy source line that referenced it, and added a note so the name does not get
changed back. The parquet example's duckdb.table(...) is the package sandbox and
is left alone.
Presto: not a supported connection type. The set is postgres, bigquery,
snowflake, trino, databricks, mysql, duckdb, motherduck, ducklake. Dropped the
sentence and retitled the section to Trino.
Config key: the examples used the deprecated top-level "projects" key. It still
works via back-compat, but it warns on every boot and this page is where readers
copy their config from, so the examples now use "environments".
Both examples on this page posted to a `queryResults/{model}` path. That path
does not exist and never has, so both 404ed regardless of the environment or
package you substituted. The endpoint is:
POST /api/v0/environments/{env}/packages/{pkg}/models/{path}/query
The JavaScript example also sent the query as a `?query=` string on a GET. The
route is POST only and reads the query from a JSON body, so that request 404ed
too.
The documented response was wrong in a way that broke the sample code: `result`
is a JSON string, not an array, so the old `return data.result` handed callers a
string where they expected rows. The example now parses it, and the page says so
explicitly. Added a note on `compactJson`, which is what produces the plain array
of row objects the page was already showing.
The examples now use the samples Publisher serves out of the box, so a reader can
run them as-is against `npx @malloy-publisher/server`. Both were verified by
extracting them from this file and executing them against a real server: the curl
returns the documented response byte for byte, and the JavaScript returns an
actual array.
The quick start said local files need no config file. They do. Following the page exactly (create a package, run npx --server_root .) produced a server with zero environments: /api/v0/environments returned [] and the package was never discovered, because Publisher only serves packages that publisher.config.json lists. There is no directory scan. The page's own diagram already showed a publisher.config.json that step 1 never told you to create. Added the missing step, and renumbered the two that follow. Other fixes on the page: - The health check and the Compose healthcheck both used a bare /status. That is not a route. The web app's catch-all answers it with 200 and an HTML page, so "curl -f http://localhost:4000/status" succeeds even when the server has loaded nothing at all, which makes the container healthcheck incapable of failing. Both now use /api/v0/status. - "List Projects" curled /api/v1/projects. There is no v1 API, so that also fell through to the web app and returned HTML rather than JSON. - Dropped the PG_CONNECT_TIMEOUT_SECONDS row. The variable was removed from Publisher along with the code that read it; the README dropped the same row already. - The Docker examples mounted packages over /publisher/packages, which is the server's own code inside the image. - Repointed a dead README#configuration anchor at docs/configuration.md. - "Restart to pick up model edits" does not work: Publisher serves a copy, so a plain restart still serves the old model. Documented --init and --watch-env, and noted that --init deletes publisher_data without prompting. - Added gs:// and s3:// to the package locations, and updated the deprecated "projects" config key and prose. Verified against @malloy-publisher/server@0.0.228 by extracting the config blocks from this file and running the page's own commands: the quick start now yields one environment and one package, where it previously yielded none.
The EmbeddedQueryResult example passed optionalPackageName and optionalProjectName. Those props were removed from the SDK in September 2025 and replaced by a single resourceUri, so the example addressed nothing and threw "Invalid URL" when the component parsed it at render. Verified against the published SDK 0.0.228: the old payload throws, the new one round-trips. They are easy to miss in a grep because both names survive as local variables inside the component. The page also said the two names are only needed when the server holds multiple projects or packages. A resource URI is always required. Other fixes: - ServerProvider takes baseURL, not server. There is no server prop, so both examples were silently ignored and the provider fell back to its default. The CORS walkthrough carried the same wrong name, which is the line most local setups copy. - The default is not http://localhost:4000. The SDK derives /api/v0 on whatever origin serves the page, which is why the example app proxies through Vite. - --server_root ./malloy-samples pointed at a directory the publisher repo does not contain; packages/server holds the config that serves the example packages the app expects. - Dropped the .env line from the project layout. VITE_PUBLISHER_API appears only in .env.example and no source file reads it; the URL comes from the Vite proxy or from baseURL. - The renderer does not pick charts on its own. Results are tables unless a render tag says otherwise. - Corrected the example app's view names to the ones it actually ships. The new example was extracted from this file and typechecked against the published SDK, then exercised at runtime.
The URL format left out the file extension. The web app routes :environmentName/:packageName/* and then branches on the extension, so /examples/storefront/storefront renders "Unrecognized file type: storefront" while /examples/storefront/storefront.malloy opens the Explorer. Both were loaded in a browser to confirm. Also named the first segment for what it is, an environment. Three of the documented hover actions do not exist in the shipped Explorer. Checked against the malloy-explorer build the SDK depends on: "Add as Sort", "Add to Query" and "Add as Nested Query" have no hits in the bundle, while "Add as order by", "Add view" and "Add as new nested query" do. A reader scanning the hover menu for "Sort" will not find it, because Malloy's concept is order by. Confirmed live as well: hovering a field shows "Add as group by", and the order by action reports "Order by is only available for fields in the output", which is a restriction the page did not mention. Lowercased the remaining labels to match what the UI actually renders.
Four corrections, three of them to text this branch itself added. The SDK embedding example did not compile. It grouped by `brand`, which belongs to the joined `products` source and is not reachable unqualified, so the query returned 400 "'brand' is not defined". The model's own top_brands view uses `products.brand`. The snippet typechecked against the SDK, which is why this survived: the types were right and the query inside them was not. Every Malloy query on these pages is now executed against the shipped storefront package. The reserved-connection-name note said the server "refuses to start" when an environment-level connection is named `duckdb`. It does not. It starts, logs the error, skips the whole environment, and keeps reporting "serving" with nothing loaded, which is the same silent failure these pages warn about two sections earlier. Corrected to the real behaviour. Removed `~/package` from the package locations. Publisher never expands a tilde (no homedir handling exists), so a `~/...` location makes the server stat a literal `~` directory and the package fails to load. The claim was pre-existing, but this branch added "resolved against --server_root" next to it, which makes it self-refuting. Dropped `cp .env.example .env` from the example-app steps, which contradicted the new line stating no environment variable points the SDK at a server, and made the JavaScript comment show the exact value the endpoint returns rather than a rounded one, since the page prints the raw float a few lines earlier on purpose.
…d claims
The environment-variable section told readers that ${VAR} keeps credentials out
of the config file, and left it there. That is true but it is not the whole
story, and the missing half is the security-relevant one: Publisher's connection
endpoints return the resolved configuration, password included, to anyone who can
reach them, and Publisher ships with no authentication and binds all interfaces.
Verified against 0.0.228 by putting a sentinel password in an environment
variable and reading it back in plaintext from an unauthenticated
GET /api/v0/environments/default/connections. Substituting a variable protects
the file and your git history, not the server, and the page now says so.
Also noted that a variable which is set but empty is not an error: only a
completely unset one fails the load, so a blank ${PG_PASSWORD} becomes a blank
password rather than a failure.
Three claims this branch added were overstated:
- The Compose healthcheck note implied /api/v0/status reports whether the server
is serving anything. It returns 200 as soon as the process is up, including
with zero environments loaded, so it checks liveness only.
- The troubleshooting step named two log strings, neither of which is emitted
when the config itself fails to parse. That case logs "Error reading
publisher.config.json", which is exactly the case a missing variable produces.
- The REST quick example works against the bundled samples only for a
zero-argument start, because any --server_root or --config disables the bundled
default config. The precondition is now stated.
Finally, the Vite proxy example forwarded /api while the surrounding prose and
the real example app both use /api/v0.
…strings More defects found reviewing this branch, most of them in text the branch itself added. The example-app walkthrough crashed. examples/data-app resolves react and @emotion from the repo-root node_modules (its vite config aliases them there), but the steps never installed at the root. Reproduced on a fresh checkout: bun run dev exits 1 with "Cannot read file" for each aliased package; with a root install present the dev server comes up on 5173 with no alias errors. The example also needs its own install, since it is not part of the workspace, so both are now listed. Use bun at the root: the repo ships only bun.lock and its engines require bun. Watch mode was documented as an alternative to --init when it is not one. Once Publisher has copied a package into publisher_data, adding --watch-env on its own does nothing: the in-place mount is only set up when that copy is absent, so edits are still ignored, and /api/v0/watch-mode/status reports "enabled": true the whole time. Verified by isolating the flag: --watch-env alone leaves the old result, --init --watch-env picks the edit up live. The instruction is now --init --watch-env. The troubleshooting strings were mapped to the wrong causes. A config that fails to parse logs "Failed to parse", never "Error reading publisher.config.json"; that second string is what an unset ${VAR} produces. A reader with a malformed config would have grepped for a string the server never prints. Both are now listed against the failure that emits them. The credential note scoped the exposure to the connection endpoints. It is wider: /api/v0/status and /api/v0/environments return the resolved password too, and /api/v0/status is the endpoint this guide tells you to health-check. Verified by reading a sentinel password back from each. The deployment section, which had no authentication caveat at all, now carries one. Also corrected two Explorer labels the earlier sweep missed: the menu item is "Nest query", and the Add Query Element list is sentence case like the rest.
The caveat this branch added was scoped to credentials, and the scoping let the
most-recommended deployment off the hook. "A Publisher that holds credentials
belongs on a trusted network" implicitly exempts the DuckDB path these guides
push hardest, the one where no connection is configured at all. That deployment
is exposed too.
Verified against the published server, unauthenticated, on a config declaring no
connections whatsoever:
POST /api/v0/environments/default/packages/pkg/connections/duckdb/sqlQuery
{"sqlStatement":"SELECT * FROM read_csv_auto('/tmp/secret-probe.csv')"}
-> 200 {"rows":[{"col":"SENSITIVE-HOST-FILE-CONTENTS"},...]}
The DuckDB sandbox every package gets automatically will read files off the
server's filesystem, so the exposure does not depend on having a database
configured.
The API is also read-write by default, which the caveat did not say. An anonymous
POST created a connection (201) and the change persists to publisher.db.
frozenConfig: true is what closes that (403), so the Mutable vs Frozen section now
names it as a security control for any Publisher reachable beyond localhost,
rather than a production nicety. It is not sufficient on its own: with
frozenConfig: true the same arbitrary SQL still runs, so the port still has to be
kept off untrusted networks.
…/status
This branch fixed the Compose healthcheck from a bare /status to /api/v0/status,
and then documented, on the connections page, that /api/v0/status serves the
resolved connection configuration with the password in it. Both statements are
true, which made the healthcheck a standing instruction to fetch database
credentials every thirty seconds and hand the output to Docker, which keeps it in
the container's health log.
/health exists in the same build, returns {"status":"UP"} and nothing else, and
works with curl -f. Verified with a sentinel password in the config: /api/v0/status
returns it, /health, /health/readiness and /health/liveness do not. The Compose
healthcheck and the Verify It Works check now use /health, and the note explains
why rather than only warning about the bare /status route.
The environment listing still shows connection configuration, so the guide keeps
recommending /api/v0/environments only as something you run yourself against your
own server, which is also what the connections page now describes.
…wrong one
The healthcheck note said /health returns just {"status":"UP"}. That is
/health/liveness's body. /health returns a liveness and readiness object with
components and groups. The point the sentence makes is unaffected, since neither
body contains connection configuration, but it quoted a response the server does
not produce, so it now describes the response rather than pinning a literal.
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.
The five Publisher pages had drifted far enough that following them mostly did not work. Every fix here was verified by running it against the published
@malloy-publisher/server@0.0.228and@malloy-publisher/sdk@0.0.228, not by reading the source. Where the two disagreed, the live behaviour won, several times.What was broken
The quick start served nothing. The page said local files need no config file. They do: Publisher only serves packages that
publisher.config.jsonlists, and there is no directory scan. Following the page exactly producedenvironments: []and the package was never discovered, while the page's own diagram showed a config file it never told you to create. Added the missing step.Both REST examples returned 404. They posted to a
queryResults/{model}path that does not exist and never has, so they failed regardless of which environment or package you substituted. The endpoint isPOST /api/v0/environments/{env}/packages/{pkg}/models/{path}/query. The JavaScript example also sent the query as a?query=string on a GET, and the route is POST only. The documented response was wrong in a way that broke the sample code:resultis a JSON string, not an array, soreturn data.resulthanded callers a string where they expected rows.The SDK example crashed at render. It passed
optionalPackageNameandoptionalProjectName, which were removed in September 2025 and replaced byresourceUri. The old payload throwsInvalid URLwhen the component parses it. Both names survive as local variables inside the component, which is why a grep does not catch this.ServerProvideralso takesbaseURL, notserver, so both provider examples were silently ignored.The Explorer URL dropped the file extension.
/examples/storefront/storefrontrendersUnrecognized file type: storefront;/examples/storefront/storefront.malloyopens the Explorer. Three documented hover actions do not exist in the shipped build: "Add as Sort", "Add to Query" and "Add as Nested Query" have no hits in the malloy-explorer bundle, while "Add as order by", "Add view" and "Add as new nested query" do.The connections page talked readers out of a working feature and into pasting credentials. It said Publisher does not support environment variable substitution and to put real credential values in the config file.
${VAR}substitution works on every string in the config. Its attached-database examples also declared an environment-level connection namedduckdb, which is reserved: that config logs an error, skips the whole environment, and leaves the server reporting"operationalState": "serving"with nothing loaded. It also documented aprestoconnection type that does not exist.The docs site documented a deleted feature.
PG_CONNECT_TIMEOUT_SECONDSwas removed from Publisher along with the code that read it. The README dropped the same row already; this brings the site back in sync.Also fixed:
/api/v1/projects(there is no v1); a bare/statushealth check that is not a route; Docker examples mounting packages over/publisher/packages, which is the server's own code inside the image; a deadREADME#configurationanchor;~/packagelocations, which never worked because Publisher does not expand a tilde; missinggs://ands3://package locations; and the example-app steps, which crashed because the app resolves react and@emotionfrom the repo-rootnode_modulesand the steps never installed there.Two things worth a maintainer's attention
Model edits need
--init. The page said to restart to pick up a change. Publisher serves a copy, so a plain restart still serves the old model.--watch-envon its own does nothing oncepublisher_dataholds that copy, and/api/v0/watch-mode/statusreports"enabled": truethe whole time it is ignoring your edits.--init --watch-envis what works.The unauthenticated API serves credentials and runs SQL. Substituting
${VAR}keeps a password out of the config file and out of git, but not out of the server:/api/v0/status,/api/v0/environmentsand the connection endpoints all return the resolved password in plaintext, and Publisher ships with no authentication and binds all interfaces. The exposure is not limited to servers that hold credentials, because the DuckDB sandbox every package gets automatically will read files off the host, so even a Publisher with no connections configured exposes its filesystem. The API is read-write by default, andfrozenConfig: truecloses config writes but not queries.This mattered for the guide itself: the Docker Compose healthcheck curled
/api/v0/status, which meant fetching database passwords every thirty seconds and handing them to Docker, which keeps healthcheck output in the container's health log. It now uses/health, which reports liveness and readiness and nothing else. The pages describe the exposure rather than implying environment variables are a security control.On
projectsvsenvironmentsThe
/api/v0/projects/...paths in these pages do not 404.server-old.tsregisters 42 legacy/projects/...routes against the same controllers so existing SDK clients keep working. The examples now useenvironmentsbecause that is the documented surface and the legacy one carries a "remove this during projects cleanup" TODO, but that change is a staleness fix, not a broken-link fix. The REST examples were broken for a different reason:queryResults/is not a route under either noun.Verification
Every command, curl, config block and code snippet was extracted from these files programmatically and executed, rather than retyped. The REST examples reproduce their documented response byte for byte, the quick start now yields one environment and one package where it previously yielded none, the SDK snippet typechecks against the published SDK and round-trips at runtime, and all four Malloy queries run against the shipped
storefrontpackage. The Explorer URLs and labels were checked in a browser and against the shippedmalloy-explorerbundle.Reviewing this branch found real defects in the fixes themselves, which is worth saying plainly. The SDK example typechecked cleanly and still returned
400 'brand' is not defined, because a Malloy query is a string and invisible to the typechecker;brandlives on the joinedproductssource. The watch-mode note documented a command that had never been run. Both were caught by executing rather than reading, which is the same reason the pages needed this work in the first place.