plugins: fix compilation if FLB_SQLDB (sqlite3) is disabled
Fixes:
- https://github.com/fluent/fluent-bit/issues/9757.
Summary by CodeRabbit
-
New Features
- SQLDB support is now optional; enabling it unlocks partial-match checklist and enhanced blob upload tracking.
- Added a new post-upload action: "Add Suffix".
-
Bug Fixes
- Non-database builds now clearly reject DB-only modes/events and fail fast when unsupported.
- Improved init and cleanup paths to prevent leftover state on failure.
-
Tests
- CI and tests run both DB-enabled and DB-disabled variants; DB-only tests are excluded when DB is off.
βοΈ Tip: You can customize this high-level summary in your review settings.
Let's make sure this compiles for all existing targets as well.
Would it be better to disable the features within the plugins that need the DB rather than the whole plugin @leonardo-albertovich ? It feels a bit like a large hammer, e.g. no tail input even if you don't use db. We would have to make the config options and any other usage conditional though so it may be worse.
Let's make sure this compiles for all existing targets as well.
How can it not? Currently, FLB_SQLDB is enabled by default, and will stay like that. The problem is the other way around, if not enabled, then compilation breaks.
Would it be better to disable the features within the plugins that need the DB rather than the whole plugin @leonardo-albertovich ? It feels a bit like a large hammer, e.g. no tail input even if you don't use db. We would have to make the config options and any other usage conditional though so it may be worse.
For me fine, but I would do that on a per feature basis. Perhaps just continue with this PR, and then fine-tune some features that might compile with some small fixups.
In general, it would be much better if all options are toggled in the automated tests, on one reference compilation system. Perhaps even by incremental builds. But either way, I just want to fix compilation now.
@patrick-stephens @edsiper I changed this PR so that plugins are still compiled, but without database support. I hope this answers https://github.com/fluent/fluent-bit/pull/10239#issuecomment-2826839994.
Tested by doing this:
cd build/
cmake -GNinja -DFLB_SQLDB=OFF ../
ninja
and
cd build/
cmake -GNinja -DFLB_PREFER_SYSTEM_LIBS=ON -DFLB_SQLDB=OFF ../
ninja
Not sure if the CI failure is relevant or something else
Not sure if the CI failure is relevant or something else
I just added #ifdef FLB_HAVE_SQLDB, which is true by default, so I don't think it's relevant.
@edsiper All merge checks seems to be passing. Can you consider this PR? This will also close #9757.
In the case of
FLB_EVENT_TYPE_BLOBSthis constant should not be removed if don't have database support.
I can do that, but where exactly should I warn about it?
Should I add one in flb_input_blob_file_register?
In the case of
filter_checklistwe should issue an error ifmodeis set topartialandFLB_HAVE_SQLDBis not defined, we cannot accept the patch as is because it would allow fluent-bit to build but cause a silent failure which would be way worse.
Is it fine to add a warning in init_config in plugins/filter_checklist/checklist.c.
In the case of
in_blobsince it cannot operate without a database it should be disabled in the build script.
But why do I see then #ifdef FLB_HAVE_SQLDB checks in the in_blob plugin if it should be disabled anyway? My commit just fixed one missing #ifdef FLB_HAVE_SQLDB which was forgotten.
See: https://github.com/search?q=repo%3Afluent%2Ffluent-bit%20path%3A%2F%5Eplugins%5C%2Fin_blob%5C%2F%2F%20FLB_HAVE_SQLDB&type=code.
@leonardo-albertovich Can you answer my questions? I'm starting to consider to just make a PR which makes sqlite3 a hard requirement. Since no one is really caring to support fluent-bit without it. @edsiper any considerations?
I'd probably be interested in an in-memory only option as well to remove more dependencies for hardening in those places where we need that and just ephemeral usage.
I'd probably be interested in an in-memory only option as well to remove more dependencies for hardening in those places where we need that and just ephemeral usage.
I don't really follow here. Please create a relevant patch set yourself. I'm only interested in fixing various compilation issues for integration with Buildroot.
Walkthrough
Gates SQLDB usage behind FLB_HAVE_SQLDB across core and multiple plugins: conditional includes, guarded DB init/insert/check/close, no-backend stubs in blob DB, updated file-part iterator signature, guarded tests and CI matrix entries; partial-match mode now requires SQLDB or init fails.
Changes
| Cohort / File(s) | Summary |
|---|---|
Checklist plugin SQLDB gatingplugins/filter_checklist/checklist.c, plugins/filter_checklist/checklist.h |
Wraps SQLDB includes, DB members and helpers with #ifdef FLB_HAVE_SQLDB; CHECK_PARTIAL_MATCH now conditional; partial-match mode errors if SQLDB disabled; guards db_insert/db_check; adds init failure cleanup. |
In-blob plugin guards & constantplugins/in_blob/blob.c, plugins/in_blob/blob.h |
Makes flb_sqldb.h include and blob_db_close() call conditional on FLB_HAVE_SQLDB; adds POST_UPLOAD_ACTION_ADD_SUFFIX = 3 macro. |
Azure Blob plugin SQLDB isolationplugins/out_azure_blob/azure_blob.c, plugins/out_azure_blob/azure_blob.h, plugins/out_azure_blob/azure_blob_conf.c |
Moves SQLDB includes and DB helpers behind FLB_HAVE_SQLDB; gates delete/register/timer functions and BLOBS event handling; non-SQLDB builds return errors for BLOBS paths; DB open/close and related mutex guarded. |
Azure BlockBlob include onlyplugins/out_azure_blob/azure_blob_blockblob.c |
Adds #include <fluent-bit/flb_utils.h>; no behavioral change. |
Blob DB no-backend stubs & API updatessrc/flb_blob_db.c |
Adds no-backend stub implementations returning NO_BACKEND_AVAILABLE when SQLDB unset; introduces flb_blob_db_lock/flb_blob_db_unlock; adds remote-id update stubs; updates flb_blob_db_file_part_get_next signature to include int *part_count. |
CI matrix.github/workflows/unit-tests.yaml |
Adds -DFLB_SQLDB=On and -DFLB_SQLDB=Off entries to unit-tests matrix. |
Tests & runtime shelltests/runtime/filter_checklist.c, tests/runtime_shell/CMakeLists.txt |
Guards mode_partial test with FLB_HAVE_SQLDB; includes in_tail_expect.sh in shell unit tests only when FLB_SQLDB is enabled. |
Sequence Diagram(s)
sequenceDiagram
autonumber
participant User as User/Config
participant Checklist as filter_checklist
participant DB as SQLDB (optional)
rect rgba(230,240,255,0.5)
Note over User,Checklist: Initialization
User->>Checklist: init_config(mode)
alt FLB_HAVE_SQLDB and mode==partial
Checklist->>DB: db_init()
else
Checklist->>Checklist: exact-match path or fail init for partial
end
end
rect rgba(235,255,235,0.5)
Note over Checklist: Loading patterns
Checklist->>Checklist: load_file_patterns()
alt partial && FLB_HAVE_SQLDB
Checklist->>DB: db_insert(pattern)
else
Checklist->>Checklist: insert into hash table
end
end
rect rgba(255,245,230,0.5)
Note over Checklist: Filtering
Checklist->>Checklist: cb_checklist_filter(record)
alt partial && FLB_HAVE_SQLDB
Checklist->>DB: db_check(record)
else
Checklist->>Checklist: hash lookup
end
end
sequenceDiagram
autonumber
participant In as in_blob
participant Out as out_azure_blob
participant DB as SQLDB (optional)
participant Timer as UploadTimer (optional)
Note over In: Exit handling
alt FLB_HAVE_SQLDB
In->>DB: blob_db_close()
else
In->>In: skip DB close
end
Note over Out: Worker init
alt FLB_HAVE_SQLDB
Out->>Timer: azb_timer_create()
else
Out->>Out: BLOBS unsupported (error)
end
Note over Out: send_blob / flush
Out->>Out: send_blob(event)
alt event==BLOBS && FLB_HAVE_SQLDB
Out->>DB: register/delete parts, manage uploads
else event==BLOBS && !FLB_HAVE_SQLDB
Out-->>Out: return FLB_ERROR (BLOBS unsupported)
else
Out-->>Out: normal non-BLOBS path
end
Estimated code review effort
π― 4 (Complex) | β±οΈ ~45 minutes
- Pay extra attention to:
src/flb_blob_db.cβ exported API changes (flb_blob_db_file_part_get_next) and new no-backend stubs.plugins/filter_checklist/*β guarded partial-mode logic and init failure cleanup.plugins/out_azure_blob/*β BLOBS gating, timer lifecycle, and error-return paths.- CI/test guards β ensure conditional test inclusion matches CI matrix.
Possibly related PRs
- fluent/fluent-bit#10919 β touches blob DB cleanup/exit paths used by
in_blob; related to guarding DB close code. - fluent/fluent-bit#10784 β modifies
src/flb_blob_db.cand SQLDB wrappers; related to API/wrapper changes here.
Suggested reviewers
- edsiper
- leonardo-albertovich
- patrick-stephens
- koleini
Poem
"I hopped through macros, quick and spry,
I fenced the DB where partials lie.
Timers hum only when SQL is found,
Exact matches keep the common ground.
A nibble, a patch β code carrots found!" πβ¨
Pre-merge checks and finishing touches
β Failed checks (1 warning)
| Check name | Status | Explanation | Resolution |
|---|---|---|---|
| Docstring Coverage | β οΈ Warning | Docstring coverage is 5.56% which is insufficient. The required threshold is 80.00%. | You can run @coderabbitai generate docstrings to improve docstring coverage. |
β Passed checks (2 passed)
| Check name | Status | Explanation |
|---|---|---|
| Description Check | β Passed | Check skipped - CodeRabbitβs high-level summary is enabled. |
| Title check | β Passed | The title accurately summarizes the main objective: adding conditional FLB_SQLDB guards to enable compilation when sqlite3 is disabled. |
β¨ Finishing touches
- [ ] π Generate docstrings
π§ͺ Generate unit tests (beta)
- [ ] Create PR with unit tests
- [ ] Post copyable unit tests in a comment
π Recent review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
π₯ Commits
Reviewing files that changed from the base of the PR and between 808154482f1b07cd586e20069f0608acb83b1670 and 37aeb405afd23148e2e5a63b8557dc2610f7f020.
π Files selected for processing (12)
.github/workflows/unit-tests.yaml(1 hunks)plugins/filter_checklist/checklist.c(8 hunks)plugins/filter_checklist/checklist.h(2 hunks)plugins/in_blob/blob.c(2 hunks)plugins/in_blob/blob.h(1 hunks)plugins/out_azure_blob/azure_blob.c(9 hunks)plugins/out_azure_blob/azure_blob.h(1 hunks)plugins/out_azure_blob/azure_blob_blockblob.c(1 hunks)plugins/out_azure_blob/azure_blob_conf.c(4 hunks)src/flb_blob_db.c(5 hunks)tests/runtime/filter_checklist.c(2 hunks)tests/runtime_shell/CMakeLists.txt(1 hunks)
β Files skipped from review due to trivial changes (1)
- plugins/out_azure_blob/azure_blob.h
π§ Files skipped from review as they are similar to previous changes (9)
- plugins/in_blob/blob.c
- tests/runtime_shell/CMakeLists.txt
- tests/runtime/filter_checklist.c
- plugins/out_azure_blob/azure_blob_blockblob.c
- .github/workflows/unit-tests.yaml
- plugins/out_azure_blob/azure_blob.c
- plugins/filter_checklist/checklist.h
- plugins/out_azure_blob/azure_blob_conf.c
- plugins/in_blob/blob.h
π§° Additional context used
π§ Learnings (1)
π Learning: 2025-08-29T06:25:27.250Z
Learnt from: shadowshot-x
Repo: fluent/fluent-bit PR: 10794
File: tests/internal/aws_compress.c:93-107
Timestamp: 2025-08-29T06:25:27.250Z
Learning: In Fluent Bit, ZSTD compression is enabled by default and is treated as a core dependency, not requiring conditional compilation guards like `#ifdef FLB_HAVE_ZSTD`. Unlike some other optional components such as ARROW/PARQUET (which use `#ifdef FLB_HAVE_ARROW` guards), ZSTD support is always available and doesn't need build-time conditionals. ZSTD headers are included directly without guards across multiple plugins and core components.
Applied to files:
src/flb_blob_db.c
𧬠Code graph analysis (1)
plugins/filter_checklist/checklist.c (3)
src/flb_filter.c (1)
flb_filter_set_context(737-740)include/fluent-bit/flb_mem.h (1)
flb_free(126-128)src/flb_sqldb.c (1)
flb_sqldb_close(129-149)
β° Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (85)
- GitHub Check: PR - container builds / Windows container images (2025)
- GitHub Check: PR - container builds / Windows container images (2022)
- GitHub Check: PR - packages build MacOS / call-build-macos-package (Intel macOS runner, macos-14-large, 3.31.6)
- GitHub Check: PR - packages build MacOS / call-build-macos-package (Apple Silicon macOS runner, macos-14, 3.31.6)
- GitHub Check: PR - packages build Linux / ubuntu/24.04.arm64v8 package build and stage to S3
- GitHub Check: PR - packages build Linux / ubuntu/24.04 package build and stage to S3
- GitHub Check: PR - packages build Linux / centos/8 package build and stage to S3
- GitHub Check: PR - packages build Linux / ubuntu/22.04 package build and stage to S3
- GitHub Check: PR - packages build Linux / raspbian/bookworm package build and stage to S3
- GitHub Check: PR - packages build Linux / debian/bullseye package build and stage to S3
- GitHub Check: PR - packages build Linux / ubuntu/22.04.arm64v8 package build and stage to S3
- GitHub Check: PR - packages build Linux / debian/trixie package build and stage to S3
- GitHub Check: PR - packages build Linux / debian/trixie.arm64v8 package build and stage to S3
- GitHub Check: PR - packages build Linux / debian/buster package build and stage to S3
- GitHub Check: PR - packages build Linux / debian/bullseye.arm64v8 package build and stage to S3
- GitHub Check: PR - packages build Linux / debian/buster.arm64v8 package build and stage to S3
- GitHub Check: PR - packages build Linux / debian/bookworm.arm64v8 package build and stage to S3
- GitHub Check: PR - packages build Linux / debian/bookworm package build and stage to S3
- GitHub Check: PR - packages build Linux / almalinux/10.arm64v8 package build and stage to S3
- GitHub Check: PR - packages build Linux / almalinux/9 package build and stage to S3
- GitHub Check: PR - packages build Linux / rockylinux/10 package build and stage to S3
- GitHub Check: PR - packages build Linux / almalinux/10 package build and stage to S3
- GitHub Check: PR - packages build Linux / almalinux/8.arm64v8 package build and stage to S3
- GitHub Check: PR - packages build Linux / almalinux/9.arm64v8 package build and stage to S3
- GitHub Check: PR - packages build Linux / rockylinux/10.arm64v8 package build and stage to S3
- GitHub Check: PR - packages build Linux / rockylinux/9.arm64v8 package build and stage to S3
- GitHub Check: PR - packages build Linux / rockylinux/8.arm64v8 package build and stage to S3
- GitHub Check: PR - packages build Linux / almalinux/8 package build and stage to S3
- GitHub Check: PR - packages build Linux / centos/9 package build and stage to S3
- GitHub Check: PR - packages build Linux / rockylinux/9 package build and stage to S3
- GitHub Check: PR - packages build Linux / centos/7.arm64v8 package build and stage to S3
- GitHub Check: PR - packages build Linux / rockylinux/8 package build and stage to S3
- GitHub Check: PR - packages build Linux / centos/8.arm64v8 package build and stage to S3
- GitHub Check: PR - packages build Linux / centos/10.arm64v8 package build and stage to S3
- GitHub Check: PR - packages build Linux / centos/10 package build and stage to S3
- GitHub Check: PR - packages build Linux / amazonlinux/2 package build and stage to S3
- GitHub Check: PR - packages build Linux / centos/9.arm64v8 package build and stage to S3
- GitHub Check: PR - packages build Linux / amazonlinux/2023.arm64v8 package build and stage to S3
- GitHub Check: PR - packages build Linux / centos/7 package build and stage to S3
- GitHub Check: PR - packages build Linux / amazonlinux/2023 package build and stage to S3
- GitHub Check: PR - packages build Linux / amazonlinux/2.arm64v8 package build and stage to S3
- GitHub Check: PR - container builds / arm/v7/production container image build
- GitHub Check: PR - container builds / arm/v7/debug container image build
- GitHub Check: PR - container builds / arm64/debug container image build
- GitHub Check: PR - container builds / amd64/debug container image build
- GitHub Check: PR - container builds / amd64/production container image build
- GitHub Check: PR - container builds / arm64/production container image build
- GitHub Check: PR - packages build Windows / call-build-windows-package (Windows 64bit (Arm64), amd64_arm64, -DCMAKE_SYSTEM_NAME=Windows -DCMA...
- GitHub Check: PR - packages build Windows / call-build-windows-package (Windows 64bit, x64, x64-windows-static, 3.31.6)
- GitHub Check: PR - packages build Windows / call-build-windows-package (Windows 32bit, x86, x86-windows-static, 3.31.6)
- GitHub Check: pr-windows-build / call-build-windows-package (Windows 32bit, x86, x86-windows-static, 3.31.6)
- GitHub Check: pr-windows-build / call-build-windows-package (Windows 64bit, x64, x64-windows-static, 3.31.6)
- GitHub Check: pr-windows-build / call-build-windows-package (Windows 64bit (Arm64), amd64_arm64, -DCMAKE_SYSTEM_NAME=Windows -DCMA...
- GitHub Check: PR - fuzzing test
- GitHub Check: run-ubuntu-unit-tests (-DFLB_SIMD=Off, 3.31.6, clang, clang++)
- GitHub Check: run-ubuntu-unit-tests (-DFLB_SQLDB=Off, 3.31.6, gcc, g++)
- GitHub Check: run-ubuntu-unit-tests (-DFLB_COMPILER_STRICT_POINTER_TYPES=On, 3.31.6, gcc, g++)
- GitHub Check: run-ubuntu-unit-tests (-DFLB_SIMD=Off, 3.31.6, gcc, g++)
- GitHub Check: run-ubuntu-unit-tests (-DFLB_SQLDB=On, 3.31.6, clang, clang++)
- GitHub Check: run-ubuntu-unit-tests (-DFLB_ARROW=On, 3.31.6, gcc, g++)
- GitHub Check: run-ubuntu-unit-tests (-DFLB_SANITIZE_MEMORY=On, 3.31.6, gcc, g++)
- GitHub Check: run-ubuntu-unit-tests (-DFLB_SQLDB=On, 3.31.6, gcc, g++)
- GitHub Check: run-ubuntu-unit-tests (-DFLB_SIMD=On, 3.31.6, clang, clang++)
- GitHub Check: run-ubuntu-unit-tests (-DFLB_SANITIZE_THREAD=On, 3.31.6, gcc, g++)
- GitHub Check: run-ubuntu-unit-tests (-DFLB_SANITIZE_MEMORY=On, 3.31.6, clang, clang++)
- GitHub Check: run-ubuntu-unit-tests (-DFLB_SANITIZE_THREAD=On, 3.31.6, clang, clang++)
- GitHub Check: run-ubuntu-unit-tests (-DFLB_SQLDB=Off, 3.31.6, clang, clang++)
- GitHub Check: run-ubuntu-unit-tests (-DFLB_COVERAGE=On, 3.31.6, gcc, g++)
- GitHub Check: run-ubuntu-unit-tests (-DFLB_SIMD=On, 3.31.6, gcc, g++)
- GitHub Check: run-ubuntu-unit-tests (-DFLB_SMALL=On, 3.31.6, gcc, g++)
- GitHub Check: run-ubuntu-unit-tests (-DSANITIZE_UNDEFINED=On, 3.31.6, clang, clang++)
- GitHub Check: pr-compile-system-libs (-DFLB_PREFER_SYSTEM_LIBS=On, 3.31.6, clang, clang++, ubuntu-22.04, clang-12)
- GitHub Check: run-ubuntu-unit-tests (-DFLB_JEMALLOC=On, 3.31.6, gcc, g++)
- GitHub Check: run-ubuntu-unit-tests (-DSANITIZE_UNDEFINED=On, 3.31.6, gcc, g++)
- GitHub Check: run-ubuntu-unit-tests (-DSANITIZE_ADDRESS=On, 3.31.6, gcc, g++)
- GitHub Check: run-ubuntu-unit-tests (-DFLB_JEMALLOC=On, 3.31.6, clang, clang++)
- GitHub Check: run-ubuntu-unit-tests (-DSANITIZE_ADDRESS=On, 3.31.6, clang, clang++)
- GitHub Check: run-ubuntu-unit-tests (-DFLB_SMALL=On, 3.31.6, clang, clang++)
- GitHub Check: pr-compile-centos-7
- GitHub Check: pr-compile-system-libs (-DFLB_PREFER_SYSTEM_LIBS=On, 3.31.6, clang, clang++, ubuntu-24.04, clang-14)
- GitHub Check: run-ubuntu-unit-tests (-DFLB_JEMALLOC=Off, 3.31.6, clang, clang++)
- GitHub Check: run-ubuntu-unit-tests (-DFLB_JEMALLOC=Off, 3.31.6, gcc, g++)
- GitHub Check: pr-compile-system-libs (-DFLB_PREFER_SYSTEM_LIBS=On, 3.31.6, gcc, g++, ubuntu-24.04, clang-14)
- GitHub Check: pr-compile-system-libs (-DFLB_PREFER_SYSTEM_LIBS=On, 3.31.6, gcc, g++, ubuntu-22.04, clang-12)
- GitHub Check: pr-compile-without-cxx (3.31.6)
π Additional comments (12)
plugins/filter_checklist/checklist.c (7)
30-127: LGTM! SQLDB helper functions are properly guarded.The static helper functions
db_init,db_insert, anddb_checkare correctly wrapped with#ifdef FLB_HAVE_SQLDBguards, ensuring they compile only when SQLDB support is enabled.
179-183: LGTM! Database insert properly guarded.The
db_insertcall forCHECK_PARTIAL_MATCHmode is correctly wrapped with#ifdef FLB_HAVE_SQLDB, ensuring it's only invoked when SQLDB support is available.
216-222: LGTM! Fail-fast behavior correctly implemented.The code now properly fails during initialization when
mode=partialis requested without SQLDB support, preventing runtime failures. The error message clearly indicates the build-time requirement.
235-242: LGTM! Database initialization properly guarded.The
db_initcall is correctly wrapped with#ifdef FLB_HAVE_SQLDBand includes proper error handling.
298-302: LGTM! Error propagation correctly implemented.The error handling now properly checks the return value from
init_configand performs complete cleanup (clearing context, freeing memory) before propagating the error. This prevents the NULL pointer dereference issue identified in previous reviews.
523-527: LGTM! Database check properly guarded.The
db_checkcall is correctly wrapped with#ifdef FLB_HAVE_SQLDB. Combined with the fail-fast initialization logic, this ensures PARTIAL_MATCH operations never execute without SQLDB support.
613-619: LGTM! Cleanup code properly guarded.The database cleanup code (finalizing statements and closing the database) is correctly wrapped with
#ifdef FLB_HAVE_SQLDB, ensuring it only executes when SQLDB support is enabled.src/flb_blob_db.c (5)
20-24: LGTM! Header includes properly guarded.The
flb_sqldb.hheader is correctly wrapped with#ifdef FLB_HAVE_SQLDB, preventing build failures when SQLDB support is disabled, while keepingflb_blob_db.hunconditionally included.
1415-1423: LGTM! Lock/unlock stubs properly implemented.The no-backend stubs for
flb_blob_db_lockandflb_blob_db_unlockconsistently returnFLB_BLOB_DB_ERROR_NO_BACKEND_AVAILABLE, matching the pattern of other stubs in this file.
1469-1474: LGTM! Remote ID update stub properly implemented.The no-backend stub for
flb_blob_file_update_remote_idhas a signature matching the SQLDB implementation and consistently returnsFLB_BLOB_DB_ERROR_NO_BACKEND_AVAILABLE.
1488-1493: LGTM! File part remote ID update stub properly implemented.The no-backend stub for
flb_blob_db_file_part_update_remote_idhas a signature matching the SQLDB implementation and consistently returnsFLB_BLOB_DB_ERROR_NO_BACKEND_AVAILABLE.
1540-1541: LGTM! Signature correctly synchronized.The no-backend stub for
flb_blob_db_file_part_get_nexthas been updated to include the newint *part_countparameter, maintaining signature consistency with the SQLDB implementation (lines 957-969).
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.
Comment @coderabbitai help to get the list of available commands and usage tips.
@leonardo-albertovich some of your remarks were fixed, can you have a look again?
@leonardo-albertovich I've updated this PR. Some new errors were fixed since. Can you have a look and eventually merge this?
@cosmo0920 Can you review this PR? Failing tests should be fixed once https://github.com/fluent/fluent-bit/pull/11077 gets merged.
This PR seems good but we're not able to add π because of failing unit testing.
Branch is rebased, remaining coderabbitai remarks are fixed. Can you merge it now? +cc @edsiper
I think in_blob, flb_blob_db must be disabled if sqlite is not available, since they must have the state.
Out S3 and out_azure will need to be aware of the lack of flb_blob_db so they can disable blob handling support
@edsiper
I think in_blob, flb_blob_db must be disabled if sqlite is not available, since they must have the state.
But why do I then see a bunch of other FLB_HAVE_SQLDB declaratives in flb_blob_db?
e.g. https://github.com/fluent/fluent-bit/blob/ed06c31991fb0c5e3f59677ceb63cabbf50ed1a7/plugins/in_blob/blob.c#L641
If it has to be disabled, do you expect me to drop all existing FLB_HAVE_SQLDB declaratives? Because I somehow expect that the original code should just compile, but was somehow broken over time. What I try to fix now.
in_blob doesn't need to be disabled because it can operate without a database
flb_blob_db does't need to be excluded because when FLB_HAVE_SQLDB isn't detected it impleemnts the exact same functions except they all return FLB_BLOB_DB_ERROR_NO_BACKEND_AVAILABLE as the error code
out_azure_blob doesn't need to be disabled because when FLB_HAVE_SQLDB isn't detected it just doesn't process blobs
out_s3 doesn't need to be disabled because when FLB_HAVE_SQLDB isn't detected it just doesn't process blobs
That's of course as far as I know and as a side note, the test case issue is unrelated to this feature because it's flb-rt-core_chunk_trace.
in_blobdoesn't need to be disabled because it can operate without a databaseflb_blob_dbdoes't need to be excluded because whenFLB_HAVE_SQLDBisn't detected it impleemnts the exact same functions except they all returnFLB_BLOB_DB_ERROR_NO_BACKEND_AVAILABLEas the error codeout_azure_blobdoesn't need to be disabled because whenFLB_HAVE_SQLDBisn't detected it just doesn't process blobsout_s3doesn't need to be disabled because whenFLB_HAVE_SQLDBisn't detected it just doesn't process blobsThat's of course as far as I know and as a side note, the test case issue is unrelated to this feature because it's
flb-rt-core_chunk_trace.
Then I don't touch anything. I would appreciate that this can just be merged then. Thanks for the review!
Oh no, I didn't review the PR, I just clarified how things worked originally and what the CI issue was.
moving this until the next milestone, some parts are still moving
@edsiper 4.2 has been released, can you merge it now?
@edsiper @leonardo-albertovich @cosmo0920 @patrick-stephens, you probably forgot about this one. Since v4.2 has been released, it's perhaps time to merge this one.
This looks good to me. However, this review comment seems not to be addressed: https://github.com/fluent/fluent-bit/pull/10239/files#r2492077239 Is it enough with this implementation?
I fixed/added some warnings for when FLB_SQLDB is not compiled. I would think its good to go now?
We fixed no left device error on our CI. So, could you rebase off current master? We need to confirm all of the CI tasks are passed before processing review.
We fixed no left device error on our CI. So, could you rebase off current master? We need to confirm all of the CI tasks are passed before processing review.
Rebased, and all tests seems to be fine. Can we merge?
@leonardo-albertovich just checking your comments were covered?
I'll review this PR again tomorrow. @ThomasDevoogdt if you have any questions that were unanswered (I think I missed some) please repeat them so I can address them tomorrow.
I'll review this PR again tomorrow. @ThomasDevoogdt if you have any questions that were unanswered (I think I missed some) please repeat them so I can address them tomorrow.
I don't have any open questions. If you see any shortcomings in this PR, then I will try to address them. But beware that I won't do any in depth refactorings, for that, someone closser to the matter should step in.
I'll review this PR again tomorrow. @ThomasDevoogdt if you have any questions that were unanswered (I think I missed some) please repeat them so I can address them tomorrow.
@leonardo-albertovich I think this PR is finalized, it would be nice having it merged. By this, the runtime tests would also start to work, limiting the chance that I have to rework this anytime soon.