chore: default OPENRAG_SHOW_SHARED_UPLOAD_TOGGLE to true#2035
Conversation
|
Important Review skippedAuto reviews are disabled on base/target branches other than the default branch. Please check the settings in the CodeRabbit UI or the ⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: CHILL Plan: Pro Run ID: You can disable this status message by setting the Use the checkbox below for a quick retry:
✨ Finishing Touches🧪 Generate unit tests (beta)
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 |
mpawlow
left a comment
There was a problem hiding this comment.
Code Review 1
- ✅ Approved / LGTM 🚀
eeb660d
into
release-saas-ga-0.6.2
…2035) (#2093) * feat: add dedicated OPENRAG_SHOW_SHARED_UPLOAD_TOGGLE flag for COS "share all" toggle (#2025) * feature Flag Share all * style: apply biome auto-fixes [skip ci] --------- Co-authored-by: github-actions[bot] <github-actions[bot]@users.noreply.github.com> * make shareall default as true (#2035) --------- Co-authored-by: github-actions[bot] <github-actions[bot]@users.noreply.github.com>
* feat: add dedicated OPENRAG_SHOW_SHARED_UPLOAD_TOGGLE flag for COS "share all" toggle (#2025) * feature Flag Share all * style: apply biome auto-fixes [skip ci] --------- Co-authored-by: github-actions[bot] <github-actions[bot]@users.noreply.github.com> * make shareall default as true (#2035) * fix: cos share all (#2050) * fix: COS shared-toggle owner not propagated on bucket sync + stale on dedupe skip Two related bugs kept "Make documents available to all users" from actually sharing documents: 1. connector_sync's bucket_filter branch (the path the COS connector UI's "Ingest N Buckets" button always hits) never forwarded body.shared to either of its two sync_specific_files calls (new files / changed files), so newly-ingested files always ended up privately owned regardless of the toggle. 2. ConnectorFileProcessor.process_item's duplicate-filename and unchanged-hash skip branches returned before resolve_shared_owner_fields() ever ran, so re-syncing a bucket whose files were already indexed left their ownership exactly as it was on the prior sync. Fix (2) via _reconcile_shared_owner(): a metadata-only update_by_query scoped to owner==self OR ownerless (same boundary as delete_document_by_filename), called from both skip paths instead of silently leaving stale ownership in place. Also fixes two pre-existing test_shared_flag.py tests that had drifted from connector_sync's current signature (missing request/session/rbac args). * test: guard is_dev_ibm_cos_enabled's default against re-regressing to enabled Found during review: is_dev_ibm_cos_enabled() previously defaulted OPENRAG_DEV_IBM_COS to "true" when unset (now fixed to "false", matching is_dev_azure_blob_enabled's pattern), which made the Enterprise/SaaS-only IBM COS connector available in every deployment by default. The existing tests only monkeypatched the function away, so they wouldn't have caught this. Add a test asserting the unset-env-var default. * style: ruff autofix (auto) --------- Co-authored-by: autofix-ci[bot] <114827586+autofix-ci[bot]@users.noreply.github.com> --------- Co-authored-by: github-actions[bot] <github-actions[bot]@users.noreply.github.com> Co-authored-by: autofix-ci[bot] <114827586+autofix-ci[bot]@users.noreply.github.com>
…#2095) * feat: add dedicated OPENRAG_SHOW_SHARED_UPLOAD_TOGGLE flag for COS "share all" toggle (#2025) * feature Flag Share all * style: apply biome auto-fixes [skip ci] --------- Co-authored-by: github-actions[bot] <github-actions[bot]@users.noreply.github.com> * make shareall default as true (#2035) * fix: cos share all (#2050) * fix: COS shared-toggle owner not propagated on bucket sync + stale on dedupe skip Two related bugs kept "Make documents available to all users" from actually sharing documents: 1. connector_sync's bucket_filter branch (the path the COS connector UI's "Ingest N Buckets" button always hits) never forwarded body.shared to either of its two sync_specific_files calls (new files / changed files), so newly-ingested files always ended up privately owned regardless of the toggle. 2. ConnectorFileProcessor.process_item's duplicate-filename and unchanged-hash skip branches returned before resolve_shared_owner_fields() ever ran, so re-syncing a bucket whose files were already indexed left their ownership exactly as it was on the prior sync. Fix (2) via _reconcile_shared_owner(): a metadata-only update_by_query scoped to owner==self OR ownerless (same boundary as delete_document_by_filename), called from both skip paths instead of silently leaving stale ownership in place. Also fixes two pre-existing test_shared_flag.py tests that had drifted from connector_sync's current signature (missing request/session/rbac args). * test: guard is_dev_ibm_cos_enabled's default against re-regressing to enabled Found during review: is_dev_ibm_cos_enabled() previously defaulted OPENRAG_DEV_IBM_COS to "true" when unset (now fixed to "false", matching is_dev_azure_blob_enabled's pattern), which made the Enterprise/SaaS-only IBM COS connector available in every deployment by default. The existing tests only monkeypatched the function away, so they wouldn't have caught this. Add a test asserting the unset-env-var default. * style: ruff autofix (auto) --------- Co-authored-by: autofix-ci[bot] <114827586+autofix-ci[bot]@users.noreply.github.com> * missing ingest settings for individual files (#2066) Root cause: FileBrowserDialog (the "Browse Files" individual-file picker) never read the "Make documents available to all users" toggle or ingest settings — it built its sync request with only selected_files, so shared silently defaulted to false on the backend. The whole-bucket ("folder") ingest path in SharedBucketView already did this correctly. Changes (frontend-only, no backend changes needed — it already threads shared uniformly): - frontend/components/file-browser-dialog.tsx: added showShared/ingestSettings props, and now includes settings and shared in the sync request body, mirroring the bucket-ingest path. - frontend/components/connectors/shared-bucket-view.tsx: passes its existing showSharedToggle/ingestSettings state down into FileBrowserDialog. --------- Co-authored-by: github-actions[bot] <github-actions[bot]@users.noreply.github.com> Co-authored-by: autofix-ci[bot] <114827586+autofix-ci[bot]@users.noreply.github.com>
…ors (backport of #2077) (#2096) * feat: add dedicated OPENRAG_SHOW_SHARED_UPLOAD_TOGGLE flag for COS "share all" toggle (#2025) * feature Flag Share all * style: apply biome auto-fixes [skip ci] --------- Co-authored-by: github-actions[bot] <github-actions[bot]@users.noreply.github.com> * make shareall default as true (#2035) * fix: cos share all (#2050) * fix: COS shared-toggle owner not propagated on bucket sync + stale on dedupe skip Two related bugs kept "Make documents available to all users" from actually sharing documents: 1. connector_sync's bucket_filter branch (the path the COS connector UI's "Ingest N Buckets" button always hits) never forwarded body.shared to either of its two sync_specific_files calls (new files / changed files), so newly-ingested files always ended up privately owned regardless of the toggle. 2. ConnectorFileProcessor.process_item's duplicate-filename and unchanged-hash skip branches returned before resolve_shared_owner_fields() ever ran, so re-syncing a bucket whose files were already indexed left their ownership exactly as it was on the prior sync. Fix (2) via _reconcile_shared_owner(): a metadata-only update_by_query scoped to owner==self OR ownerless (same boundary as delete_document_by_filename), called from both skip paths instead of silently leaving stale ownership in place. Also fixes two pre-existing test_shared_flag.py tests that had drifted from connector_sync's current signature (missing request/session/rbac args). * test: guard is_dev_ibm_cos_enabled's default against re-regressing to enabled Found during review: is_dev_ibm_cos_enabled() previously defaulted OPENRAG_DEV_IBM_COS to "true" when unset (now fixed to "false", matching is_dev_azure_blob_enabled's pattern), which made the Enterprise/SaaS-only IBM COS connector available in every deployment by default. The existing tests only monkeypatched the function away, so they wouldn't have caught this. Add a test asserting the unset-env-var default. * style: ruff autofix (auto) --------- Co-authored-by: autofix-ci[bot] <114827586+autofix-ci[bot]@users.noreply.github.com> * missing ingest settings for individual files (#2066) Root cause: FileBrowserDialog (the "Browse Files" individual-file picker) never read the "Make documents available to all users" toggle or ingest settings — it built its sync request with only selected_files, so shared silently defaulted to false on the backend. The whole-bucket ("folder") ingest path in SharedBucketView already did this correctly. Changes (frontend-only, no backend changes needed — it already threads shared uniformly): - frontend/components/file-browser-dialog.tsx: added showShared/ingestSettings props, and now includes settings and shared in the sync request body, mirroring the bucket-ingest path. - frontend/components/connectors/shared-bucket-view.tsx: passes its existing showSharedToggle/ingestSettings state down into FileBrowserDialog. * fix: introduces a user-facing duplicate handling workflow for bucket-based connectors (#2077) * fix japanese file names Root cause: COS/Azure/S3 connectors build document.id as the raw, non-ASCII object key ("bucket::報告書.pdf"), and that value was reused verbatim as document_id in the default Langflow ingestion path — the only ingestion path that puts non-ASCII text into ASCII-only HTTP headers and an unbounded-length OpenSearch identifier. Google Drive (opaque ASCII ID) and manual upload (content hash) were structurally immune; COS/Azure/S3 were the only paths exposed. Fix applied (matches the already-proven split used by the non-Langflow ingestion path): - document_index_writer.py — DocumentIndexContext gained a connector_file_id field, now written onto every indexed chunk when present. - langflow_file_service.py — _resolve_document_id now hashes any supplied connector_file_id into a stable ASCII document_id, instead of using it verbatim; connector_file_id threads through run_ingestion_flow → _configure_ingest_callback → the JWT context. - processors.py — the Langflow-path connector branch now passes connector_file_id=document.id (not document_id=document.id), and the post-ingest existence check / metadata sync now match on connector_file_id. - connectors/service.py — stale comments corrected (dedupe/ACL-sync already matched both fields, no logic change needed there). - Added 13 new unit tests (test_resolve_document_id_connector_file_id.py, test_document_index_writer_connector_file_id.py, test_bucket_connector_unicode_file_ids.py) — confirmed they fail on the old code and pass on the fix. - Ran the full unit suite: same 14 pre-existing failures before and after (unrelated to this change — auth/settings/encryption tests), no new regressions. * style: ruff autofix (auto) * fix connector File name issue * ingest fix * Update processors.py * review fix * fix connector dialog on rewrite * overwrite dialog box * Update connectors.py --------- Co-authored-by: autofix-ci[bot] <114827586+autofix-ci[bot]@users.noreply.github.com> --------- Co-authored-by: github-actions[bot] <github-actions[bot]@users.noreply.github.com> Co-authored-by: autofix-ci[bot] <114827586+autofix-ci[bot]@users.noreply.github.com>
Flips the default of
OPENRAG_SHOW_SHARED_UPLOAD_TOGGLEfromfalsetotrue, so the COS "Make documents available to all users" (share-all) toggle is shown out of the box.Why
The flag was introduced in #2025 to expose the share-all toggle independently of
OPENRAG_SHOW_PROVIDER_INGEST_SETTINGS(which gates the rest of the per-upload ingest tuning knobs). It defaulted tofalse, so in SaaS / IBM-auth deployments the toggle was hidden unless the env var was explicitly set — the common case wanted it visible. Making it on by default removes that per-deployment setup step.Behavior
OPENRAG_SHOW_SHARED_UPLOAD_TOGGLE=false.OPENRAG_SHOW_PROVIDER_INGEST_SETTINGS— the two flags remain independent.Changes
src/config/settings.py— default"false"→"true", updated comment.tests/unit/config/test_show_shared_upload_toggle.py— updated default-value test and added a case for explicitly disabling via=false.