[pull] master from cube-js:master - #695
Merged
Merged
Conversation
* docs: fix five stale documentation sections - default_ui_filters now seed queries in the Google Sheets / Excel add-in as well as workbooks and React Embed SDK views - document one exploration placed on many sheets/workbooks in the Excel/Sheets add-ins - document the measure position and order controls in the Excel/Sheets pivot builder - document runQuery's branchName parameter and the branch-verification step in the MCP server workflow - allowExport in embedded dashboards now covers PNG/PDF, not just CSV * docs: fix contradictory wording flagged in review - allowExport: replace the confusing "opt-in ... otherwise opt-out" framing with a plain list of what stays hidden - saved reports: clarify that an anchor is a fixed cell reference, so row/column inserts above it break the placement instead of shifting with it --------- Co-authored-by: Claude <noreply@anthropic.com>
* fix(cubestore-driver): make close() actually close the connection `close()` called `webSocket.close()` and left it there, which does not release anything: the 'close' handler re-sends whatever was in flight over a fresh connection, and the 5s heartbeat on that one keeps the socket -- and everything reachable from the interval, including the orchestrator holding the driver -- alive for the life of the process. A query arriving after the close re-opened the connection just as readily. A single `closed` flag makes the close terminal. `initWebSocket()` refuses once it is set, which covers both the late query and the re-send path, whose `catch` then fails the messages that were in flight instead of re-opening for them. Messages already in flight are still given the chance to be answered -- an eviction can land mid-query and Cube Store may already be working on the answer -- so the socket goes away once the last of them settles, or immediately if there are none. A query Cube Store never answers is bounded by the existing no-heartbeat timeout, which closes the socket and lands on the same path. Alongside it, two things that leaked a connection nobody was waiting for: - The heartbeat interval is now stopped through `teardown()` wherever the socket dies, not only in the 'close' handler. It is the timer, not the socket, that keeps the graph reachable. - The 'error' handler no longer reconnects once the socket has been established. Retrying is only useful while somebody is still waiting for `readyPromise`; after that it raised a connection no caller asked for and kept it alive on its own heartbeat, while what was in flight is re-sent by the 'close' handler anyway. The re-send itself now also bails out when everything it was scheduled for has settled in the meantime. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01HF1JQbLKkZ4XLd1hLXYVmA * fix(server-core): release orchestrators evicted from the LRU `OrchestratorStorage` keeps `OrchestratorApi` instances in an `LRUCache` with no `dispose` handler, so an entry pushed out by `max` -- or replaced, deleted, expired -- is dropped without `release()` ever being called. Its drivers stay open, and with them the Cube Store WebSocket. Only `releaseConnections()` at shutdown released anything. Nothing else closes that socket: the driver pings it every 5s, so neither side times it out, and that live timer keeps the whole orchestrator reachable from a GC root. A collected socket would not be closed anyway -- V8 runs no destructors and neither `ws` nor `net` registers a finalizer. In Node a socket is closed explicitly or never. So a deployment whose `contextToOrchestratorId` has high cardinality (per-user ids, or ids that rotate on a timer) leaked one Cube Store connection per eviction. Measured in production: ~450 new sockets/minute per API pod (429 on a pod 43s after restart, 5,198 at 16 min; a pod serving no traffic held 1), 65,551 established sockets on the router they pointed at, and that region's ingress OOM-killed 38-46 times in six hours. Reproduced on deployments pinned to v1.7.2 and v1.7.7, so it is not version-specific. `disposeAfter` covers every removal reason. It is `disposeAfter` rather than `dispose` because `release()` is async and calls into the drivers, while `dispose` runs synchronously inside `set()`/`delete()`. The scheduled releases are tracked in `pendingReleases` so `releaseConnections()` can await them: `clear()` only schedules them, so without that shutdown could return with the connections still open. A release that fails is swallowed rather than left as an unhandled rejection, and each entry removes itself from the set when it finishes. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01HF1JQbLKkZ4XLd1hLXYVmA * fix(cubestore-driver): bound the drain a close waits out Review follow-ups on the `closed` flag. The drain was unbounded and re-checked only from the 'message' handler, so a message Cube Store never answers kept the socket -- and the heartbeat interval that makes it reachable -- for the life of the process. The no-heartbeat close does not save it: Cube Store keeps answering the pings of a connection whose query simply never completes, so nothing would call `closeIfDrained()` again. That is the leak this branch exists to close, behind a narrower door. `close()` now arms a bound; when it fires, whatever is left is rejected naming the count, and the socket is torn down. A stuck message costs one query instead. `closeIfDrained()` also took the socket the answer arrived on rather than reading `this.webSocket`: a late answer on a socket the re-send path has already superseded was being read as the current connection having drained. Dropped the `established` flag as well. It existed to stop the 'error' handler reconnecting a socket nobody was waiting for any more, but with `closed` making the close terminal that reconnect is no longer orphaned -- it is assigned to `this.webSocket`, so it is reachable and the close reaps it. The heartbeat is still stopped there, since a 'close' does not always follow an 'error'. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01HF1JQbLKkZ4XLd1hLXYVmA * fix(server-core): log an orchestrator release that fails `OrchestratorStorage` swallows the rejection so that one orchestrator failing to close cannot fail shutdown -- which also means nothing said so. On the eviction path there was never a caller to tell, and at shutdown `releaseConnections()` used to propagate the error and no longer can. A driver that fails to close takes its connection with it, and that is the one signal an operator has that this fix is not working in their deployment, so `release()` logs it through the api's own logger before rethrowing. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01HF1JQbLKkZ4XLd1hLXYVmA * fix(cubestore-driver): bound the close in the heartbeat, not a second timer The bound on how long a closed connection waits for its in-flight messages was its own `setTimeout`. The heartbeat interval is already the timer that owns "this socket has waited long enough", and it is already the thing keeping the socket reachable, so the check belongs there: one timer to reason about, and no chance of a stray one outliving the socket it was bounding. The interval keeps pinging while a closed connection drains, so one that is draining legitimately is not dropped for inactivity, and the closed branch returns before the ordinary no-heartbeat check -- that check cannot bound this wait, since Cube Store keeps answering the pings of a connection whose query is never completed. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01HF1JQbLKkZ4XLd1hLXYVmA * fix(server-core): don't build the external driver just to close it `release()` called `releaseDriver(this.options.externalDriverFactory)` unconditionally, and that factory *creates* the connection on first call -- `server.ts` builds the driver and runs `testConnection()` inside it. Harmless while release only happened at shutdown; with a release on every LRU eviction it means an orchestrator that never touched Cube Store opens a connection purely to close it, which is churn against exactly the router this branch exists to relieve, at the eviction rate the PR describes. The factory is wrapped in the constructor -- before `QueryOrchestrator` captures it off `options` -- so the release can tell whether anything built the driver, and the flag is set on fulfilment: `server.ts` releases the driver and clears its memo when `testConnection()` fails, so a build that threw left nothing open. The release closes through the unwrapped factory, or it would set the flag again on the way out. The data-source loop was already guarded this way by `seenDataSources`. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01HF1JQbLKkZ4XLd1hLXYVmA * fix(cubestore-driver): don't leave an ownerless rejection on the retry path Dropping the `established` flag left a fatal hole, and the reasoning in that commit message was incomplete: it covered a live connection, where the retry's reconnect lands in `this.webSocket` and a later close reaps it, and missed the closed one, where there is no reconnect to reap -- only a rejection with no owner. `initWebSocket()` refuses once `closed` is set, and the 'error' handler handed that rejected promise to `resolve`. On a socket whose `readyPromise` has already settled, `resolve` returns without adopting what it was given, so nothing ever observes the rejection -- and an unhandled rejection is fatal under Node's default `--unhandled-rejections=throw`, which would take the API pod with it. Two changes, each standing on its own: - The handler returns early when the connection is closed. There is nothing to retry towards, and this also stops arming a pointless timer per evicted connection. - The retry uses `.then(resolve, reject)` rather than `resolve(promise)`, so any rejection is observed whether or not `readyPromise` has settled. The early return covers the case above, but a close landing between the timer being armed and firing does not go through it, and neither would anything else `initWebSocket()` throws. The test emits the 'error' rather than provoking one: `ws` reports a dying socket as 'close' against the mock, and the errors that do reach this handler in the wild -- a protocol error on an established socket, or a connect retry still pending from before the close -- are not reproducible there. The handler's contract is what is being pinned, and the test fails against the previous commit with exactly the fatal rejection. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01HF1JQbLKkZ4XLd1hLXYVmA * Revert "fix(server-core): don't build the external driver just to close it" This reverts commit c22a475, keeping only the release logging. The guard is not worth its complexity. For any orchestrator that served a query the external driver has already been built -- the Cube Store queue and cache go through the same factory -- so the guard only saves an open/close for one that was created and never used, and the wrapper it needs drew two defects of its own in review. Master's unconditional release stays. For the record, since it is the reason the guard existed: the operation it skips is not free. `CubeStoreDriver.testConnection()` runs `SELECT 1`, which opens the socket, so releasing a never-built external driver does connect + query + close. That cost is now accepted for the minority of evictions where it applies. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01HF1JQbLKkZ4XLd1hLXYVmA * fix(cubestore-driver): settle readyPromise when a closed connection errors The early return added in 52abc59 skips every path that settles `readyPromise`: `'open'` will not fire, `teardown()` above has stopped the interval, the 'close' handler does not touch it, and the retry that used to settle it -- by adoption, since `resolve(promise)` on an unsettled promise adopts -- is exactly what the early return skips. So a socket still CONNECTING when the connection is closed left the `await this.initWebSocket()` in `sendMessage` pending for good. That is an eviction landing while an orchestrator's first query is connecting, and it turned the rejection that path used to produce into a request hung on nothing, which is worse than the failure it replaced. Rejecting before the return restores it, and is a no-op once the socket has opened. Also corrected the comment above it: what was written to the socket is rejected by the 'close' handler that `ws` emits after an 'error', and *not* by the drain bound, which cannot fire any more because `teardown()` has stopped the interval that evaluates it. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01HF1JQbLKkZ4XLd1hLXYVmA * refactor(cubestore-driver): make two close-path fields mean what they say Neither is a defect; both were noted in review as harmless and are cheaper to remove than to keep explaining. `currentConnectionTry` was incremented above the closed branch, i.e. on a path that returns without attempting a connection. It now counts only real tries, which is what paces the retries and spends the `maxConnectRetries` budget a cold connect needs. `closedAt` was re-stamped by every `close()`, so a second call on a draining connection restarted the bound. That is reachable: `release()` closes the data-source and external drivers separately, which is the same connection twice when one `CubeStoreDriver` backs both. It now records when the close was first asked for. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01HF1JQbLKkZ4XLd1hLXYVmA --------- Co-authored-by: Claude <noreply@anthropic.com>
) * docs: document embed header controls and standalone explorations - Add URL parameters for hiding individual embedded-dashboard header controls (title, back button, edit, duplicate, or the whole bar), with a pointer from the Creator Mode page. - Clarify that a saved item without a workbook is a standalone exploration (the same object as a workbook report, just unfiled), documented consistently across Analytics Chat, Explore, Workbooks, Python analysis, and the MCP server's createReport tool. * docs: tighten wording and drop an unverified add-in claim - Simplify the allowExport comparison and drop CSS-level detail in favor of the actual precedence rule between showDashboardHeader and the per-control parameters. - Call out that the header-control parameters are URL-only, not part of the Generate Session settings object, with a matching pointer from the API reference. - Remove a claim that a saved Python analysis can be opened in Excel or Google Sheets — the add-ins render a report's raw SQL result, not a Python script's output, so that combination doesn't work. - Trim a features-list bullet that had grown a how-to prompt inside it. * docs: align the Python analysis feature bullet with the exploration terminology Same list on the same page already said saved results can be a report or a standalone exploration; the Python bullet still said "workbook" only. * docs: fix a dangling pronoun reference in the Python save sentence * docs: consolidate two duplicated callouts into one * docs: fix a pronoun reaching past its intended antecedent --------- Co-authored-by: Claude <noreply@anthropic.com>
#11668) * docs(api-reference): sync Platform API docs for platform-client v0.4.0 Regenerates api.yaml, docs.json and introduction.mdx from the cubejs-enterprise public OpenAPI spec, and changelog.mdx from the platform-client CHANGELOG.md, via scripts/extract-api.mjs and extract-changelog.mjs. Adds the new Region Private Links group to TAG_ORDER so it's placed after Regions instead of appended A-Z. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01XdLcEJjqSB78Zu4zdbo5yn * fix(api-reference): pick up the corrected platform-client v0.4.0 changelog Re-syncs changelog.mdx after cubedevinc/cubejs-enterprise#14466 fixed three inaccuracies in the release notes flagged by review: users/groups already had cursor pagination in 0.3.0 (search is what's new), the DeploymentEnvironment(Tokens)ListResponse.pagination narrowing is a TS break, and the report-placement field additions were unlisted. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01XdLcEJjqSB78Zu4zdbo5yn * fix(api-reference): exclude Cube-staff-only Regions/PrivateLinks ops from public docs Per review: every new Regions/PrivateLinks operation (create/get/update/ destroy/apply/provisioning-status/cloud-provider-catalog on regions, and all 5 PrivateLink ops) is gated "Cube super admin only" — no reader of these docs can call any of them. EXCLUDE_OPERATIONS already exists for exactly this class of route ("Account-level / internal admin APIs kept out of the public docs"), and no prior operation in this spec carried that banner, so this adds all 12 to it rather than leaving the sidebar's first customer-uncallable pages in by default. GET /api/v1/regions (listRegions) is untouched — any tenant can call it. The Region Private Links tag now has zero operations left, so it drops out of the nav automatically; reverts the TAG_ORDER entry added for it. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01XdLcEJjqSB78Zu4zdbo5yn * fix(api-reference): pick up the second platform-client v0.4.0 changelog correction Re-syncs after cubedevinc/cubejs-enterprise#14466's follow-up fix: the report-input field list is now split per schema (Connect/Create/Update gained different subsets), and ResourceGroupPolicyDto joins its sibling InheritedGroupPolicyDto for the name/userCount addition. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01XdLcEJjqSB78Zu4zdbo5yn * fix(api-reference): pick up the isStagingEnvironmentEnabled changelog note Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01XdLcEJjqSB78Zu4zdbo5yn * fix(api-reference): fail the sync when an EXCLUDE_OPERATIONS entry matches nothing Per review: the exclusion lookup silently no-ops if an entry's method+path stops matching (upstream rename/move), and --check can't catch that either — both the committed and freshly generated output would contain the leak. That's low-stakes for "stray/incomplete admin routes" but more load-bearing now that 12 entries carry a privacy intent (keeping Cube-staff-only Regions/PrivateLinks operations out of the public docs). Track which entries matched during the path loop and abort with the unmatched ones listed if any didn't. This immediately caught a real stale entry: `GET /api/v1/ai-engineer/ active-region` was removed from the source spec as a breaking change in platform-client 0.3.0, so its exclusion had already gone dead — removed. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01XdLcEJjqSB78Zu4zdbo5yn * fix(api-reference): pick up the Regions/PrivateLinks exclusion note Re-syncs after cubedevinc/cubejs-enterprise#14466 added a note that these SDK-typed operations are excluded from this reference (EXCLUDE_OPERATIONS). Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01XdLcEJjqSB78Zu4zdbo5yn * fix(api-reference): pick up the GET /api/v1/regions/ clarification Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01XdLcEJjqSB78Zu4zdbo5yn * fix(api-reference): pick up removal of the Regions/PrivateLinks changelog section Re-syncs after cubedevinc/cubejs-enterprise#14466 dropped the section per direction not to surface or mention Cube-staff-only endpoints anywhere, including the SDK changelog. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01XdLcEJjqSB78Zu4zdbo5yn * fix(api-reference): auto-detect and exclude Cube-staff-only operations from the routing Per direction: instead of hand-maintaining a list of super-admin-gated paths in EXCLUDE_OPERATIONS (which only protects endpoints someone remembered to add, and only re-validates the ones already listed), detect them structurally at generation time. console-server prefixes every super-admin-gated operation's OpenAPI description with SUPER_ADMIN_ONLY_DOC_MARKER (see cubedevinc/ cubejs-enterprise#14467 for the cross-reference comment on that constant). Matching on that exact text in the path loop — before applyDescription() moves the description into x-mint.content — catches any current or future staff-only operation automatically, with no per-endpoint bookkeeping in this repo. Removes the 12 manually-listed Regions/PrivateLinks entries from EXCLUDE_OPERATIONS (now redundant and would trip the fail-closed assertion from d24cdd8, since the auto-detection deletes them first). --check confirms byte-identical output to the hand-maintained list. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01XdLcEJjqSB78Zu4zdbo5yn * fix(api-reference): fail closed if the super-admin marker stops matching Per review: the marker-based auto-exclusion added in ab7fc3a is an exact-substring match against text authored in a different repo, so it fails OPEN (not closed) if that text drifts upstream — the opposite of what d24cdd8 just bought for the old hand-maintained list. Two guards restore fail-closed without depending on the marker staying exact: abort if the marker matched zero operations (today's spec always has staff-only ones), and a residual scan over every kept operation's rendered content for "super admin" (looser than the marker itself, specifically excluding the unrelated-but-similar 🔒 "Admin only" marker, which is legitimately public). Verified both actually fire: a marker that matches nothing aborts, and the real output is unaffected (byte-identical api.yaml/docs.json/introduction.mdx). Also narrowed the marker itself to a short, stable anchor ("Cube super admin only") rather than the full two-sentence constant, which is more exposed to upstream rewrapping — safe now that the residual scan is the actual backstop either way. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01XdLcEJjqSB78Zu4zdbo5yn * fix(api-reference): close partial-drift gap in the super-admin residual scan Widen the residual-scan regex from /super admin/i to /super[-\s]?admins?\b/i so a hyphenated "super-admin" rewording on a newly-added operation still trips the guard even when autoExcludedCount stays non-zero (the floor guard alone can't catch a partial drift, only a total reword). --------- Co-authored-by: Claude <noreply@anthropic.com>
* docs: document the sankey chart type * docs: correct the sankey tooltip default and register sankey in shared chart lists --------- Co-authored-by: igorlukanin <3852894+igorlukanin@users.noreply.github.com>
…11676) * docs: recipe for cross-data-source UNION ALL queries via the SQL API * docs: correct cube-name resolution note, flag non-additive measures, fix recipe count * docs: rewrap the pg_ prefix note so no inline code span breaks across lines --------- Co-authored-by: igorlukanin <3852894+igorlukanin@users.noreply.github.com>
github-actions
Bot
added
pr:community
rust
data source driver
cubejs-jdbc-driver
driver:athena
driver:bigquery
driver:clickhouse
driver:crate
driver:databricks
driver:dremio
driver:druid
driver:duckdb
driver:firebolt
driver:hive
driver:materialize
driver:mongodb
driver:mssql
driver:mysql
driver:mysql-aurora-serverless
labels
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 subscribe to this conversation on GitHub.
Already have an account?
Sign in.
4 participants
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.
See Commits and Changes for more details.
Created by
pull[bot] (v2.0.0-alpha.4)
Can you help keep this open source service alive? 💖 Please sponsor : )