feat: add feedback banner and issue links to lifecycle errors - #16
Merged
Conversation
Scenario lifecycles interact with real schemas and constraints we cannot reproduce here, so the users who hit a problem are the only ones who can tell us what their setup looked like. Two surfaces now point at the tracker. A once-a-day banner on interactive load, from the shared DataZooDE/duckdb-extension-banner submodule. Silent when piped, in notebooks, in CI and under the test runner, so no existing expected output changes. An issue link on errors from the lifecycle entry points -- refresh, list, merge and migrate -- via DATAZOO_GUARD at their registration sites. The scan/bind pointers reach the registration helpers as runtime parameters, so they are guarded where they are named rather than inside the helper. Errors DuckDB raises before dispatch -- binder, catalog, arity -- are left unannotated; they are not ours. feedback.test pins that. The banner's telemetry flag follows TELEMETRY_SUPPORTED and is set after that variable is computed: placing it earlier silently read an empty value and would have disabled banner_shown on every platform. Suite green: 1149 assertions in 27 cases.
This was referenced Aug 6, 2026
The banner library declared INTERFACE cxx_std_17. That propagates through the extension target into DuckDB's own tools/plan_serializer, which then compiles as C++17 while libduckdb_static stays C++11 -- BufferedFileWriter::DEFAULT_OPEN_FLAGS ends up COMDAT-weak on one side and strong on the other, and the link fails with a multiple definition error. anofox-statistics CI caught it; the declaration has been removed upstream since every consumer already builds at C++17. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
It sat inside #ifdef HAS_POSTHOG_TELEMETRY, so any build without telemetry -- MinGW, wasm, cross-compiled macOS -- would not see datazoo::ShowBanner at all and fail to compile. Local builds have telemetry on, which is why this passed here. Same mistake as the erpl-tunnel SSH-only break: an include placed after the last #include in the file rather than at genuine top level. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
On MinGW the 'struct _stat64' tag hides the function of the same name in C++, so the banner's _stat64 call parsed as a constructor and every rtools/MinGW Windows job failed to compile. The shared library now uses plain stat() everywhere except MSVC. Caught by anofox-similarity CI, which builds windows_amd64_rtools.
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.
Scenario lifecycles interact with real schemas and constraints we cannot reproduce here, so the users who hit a problem are the only ones who can tell us what their setup looked like.
Part of a fleet-wide rollout using the shared
DataZooDE/duckdb-extension-bannersubmodule. Same family as erpl-adt#36 (merged), erpl-tunnel#2, quack-oauth#12, erpl-idoc#7, erpl-rev#63, anofox-statistics#127.What changed
Once-a-day banner on interactive load. Silent when piped, in notebooks, in CI and under the test runner — which is why no existing expected output changes.
Issue link on errors from the lifecycle entry points —
refresh,list,merge,migrate— viaDATAZOO_GUARDat their registration sites.The scan/bind pointers reach the registration helpers as runtime parameters, so they are guarded where they are named rather than inside the helper.
Errors DuckDB raises before dispatch (binder, catalog, arity) are left unannotated; they are not ours.
feedback.testpins this.One CMake detail worth noting
DATAZOO_BANNER_TELEMETRYfollowsTELEMETRY_SUPPORTEDand is set after that variable is computed. Placing the block earlier (where the include directories live) silently read an empty value and would have disabledbanner_shownon every platform while still looking correct.Verified
feedback.test(5 assertions)Need help on this PR? Tag
@codesmith-botwith what you need. Autofix is disabled.