Skip to content

[HOTFIX] Honor ENABLE_HIGHLIGHT_API_DEPLOYMENT env var on on-prem#1933

Merged
vishnuszipstack merged 1 commit into
v0.161.4-hotfixfrom
fix/onprem-highlight-api-deployment
Apr 29, 2026
Merged

[HOTFIX] Honor ENABLE_HIGHLIGHT_API_DEPLOYMENT env var on on-prem#1933
vishnuszipstack merged 1 commit into
v0.161.4-hotfixfrom
fix/onprem-highlight-api-deployment

Conversation

@vishnuszipstack
Copy link
Copy Markdown
Contributor

@vishnuszipstack vishnuszipstack commented Apr 28, 2026

Summary

  • Hotfix on top of v0.161.4. Pairs with cloud hotfix Zipstack/unstract-cloud#1467.
  • Wraps ENABLE_HIGHLIGHT_API_DEPLOYMENT env-var read in backend/backend/settings/base.py with CommonUtils.str_to_bool(...) so values like False / false / 0 actually evaluate falsy. Previously os.environ.get(...) returned the raw string, so any non-empty value (including "False") was truthy in Python and bypassed the gate.
  • The setting is consumed as the ConfigSpec.default in unstract-cloud/backend/plugins/configuration/cloud_config.py. Configuration.get_value_by_organization(...) returns this default when no per-org row exists, so the bool fix takes effect on cloud and on-prem builds where the configuration plugin is loaded.

Why this is the only OSS change

  • The on-prem regression fix (loading the configuration plugin in on-prem images so the registry knows the key) is cloud-only and ships in Zipstack/unstract-cloud#1467.
  • This OSS PR is intentionally minimal — only the bool parsing.

Test plan

  • On-prem (with cloud PR UN-2653 [FIX] Fixed issue in JSON structure parsing #1467 merged & on-prem image rebuilt): set ENABLE_HIGHLIGHT_API_DEPLOYMENT=true, run an API deployment (sync + polled), confirm highlight_data and extracted_text are present in the response metadata.
  • On-prem: set ENABLE_HIGHLIGHT_API_DEPLOYMENT=False, confirm both keys are stripped (this is the case the bool fix unblocks).
  • Cloud: with no per-org Configuration row and env=true, confirm fleet-wide default keeps highlight data.
  • Cloud: with no per-org row and env=False, confirm fleet-wide default strips both keys.

🤖 Generated with Claude Code

@coderabbitai
Copy link
Copy Markdown
Contributor

coderabbitai Bot commented Apr 28, 2026

Important

Review skipped

Auto reviews are disabled on base/target branches other than the default branch.

Please check the settings in the CodeRabbit UI or the .coderabbit.yaml file in this repository. To trigger a single review, invoke the @coderabbitai review command.

⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro

Run ID: 42c1ce7f-e464-4021-8687-3ccfdac5cbe7

You can disable this status message by setting the reviews.review_status to false in the CodeRabbit configuration file.

Use the checkbox below for a quick retry:

  • 🔍 Trigger review
✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch fix/onprem-highlight-api-deployment

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.

❤️ Share

Comment @coderabbitai help to get the list of available commands and usage tips.

@greptile-apps
Copy link
Copy Markdown
Contributor

greptile-apps Bot commented Apr 28, 2026

Greptile Summary

The PR changes only backend/backend/settings/base.py, wrapping the ENABLE_HIGHLIGHT_API_DEPLOYMENT env var read in CommonUtils.str_to_bool so that string values like "False" or "false" evaluate falsy rather than truthy.

  • The base.py fix is correct and consistent with the existing DASHBOARD_BUCKET_CACHE_ENABLED pattern in the same file.
  • However, neither gate site (deployment_helper.py:263 nor api_deployment_views.py:221) reads settings.ENABLE_HIGHLIGHT_API_DEPLOYMENT; both default enable_highlight = False and only override via ConfigurationRegistry.is_config_key_available(). On on-prem the registry never registers the key, so the parsed base.py setting is never consulted and the env var remains effectively ignored at runtime. The PR description states fixes were applied to both gate sites, but those changes are absent from this diff. Previous reviews have flagged this gap in both files.

Confidence Score: 4/5

The base.py change is safe in isolation, but the PR's stated goal of fixing on-prem highlight data is not achieved because the gate sites do not read settings.ENABLE_HIGHLIGHT_API_DEPLOYMENT.

Only P2 findings in the changed file itself; however, the PR description claims fixes at both gate sites that are absent from the diff, meaning the on-prem regression flagged in prior reviews remains unresolved.

backend/api_v2/deployment_helper.py and backend/api_v2/api_deployment_views.py — both gate sites still default enable_highlight = False on on-prem and do not fall back to settings.ENABLE_HIGHLIGHT_API_DEPLOYMENT.

Important Files Changed

Filename Overview
backend/backend/settings/base.py Wraps env var read in CommonUtils.str_to_bool; correctly handles "true"/"false" strings, but "1" now evaluates to False (previously truthy), and neither gate site reads this setting on on-prem.

Flowchart

%%{init: {'theme': 'neutral'}}%%
flowchart TD
    A[Request hits gate site] --> B{ConfigurationRegistry\nis_config_key_available?}
    B -- "Yes (Cloud)" --> C[get_value_by_organization\nfor org-level override]
    B -- "No (On-prem)" --> D[enable_highlight = False\nHardcoded default]
    C --> E{enable_highlight?}
    D --> E
    E -- "False" --> F[Strip highlight_data\n& extracted_text]
    E -- "True" --> G[Include highlight_data\n& extracted_text]
    H[settings.ENABLE_HIGHLIGHT_API_DEPLOYMENT\nbase.py - this PR] -. not read by gate sites .-> D
    style D fill:#f96,color:#000
    style H fill:#ff9,color:#000
Loading

Fix All in Claude Code

Prompt To Fix All With AI
This is a comment left during a code review.
Path: backend/backend/settings/base.py
Line: 692-694

Comment:
**`"1"` silently evaluates to `False` after this change**

`CommonUtils.str_to_bool` only returns `True` for `"true"` (case-insensitive). A user who set `ENABLE_HIGHLIGHT_API_DEPLOYMENT=1` (a common Unix convention) would have had it work before this PR (non-empty string is truthy) and silently break after it. The PR description only mentions making `"False"/"false"/"0"` falsy — `"1"` is not mentioned. Consider accepting `"1"` as truthy as well, consistent with how the similar `ConfigurationRegistry.get_value_by_organization` path parses booleans.

```suggestion
ENABLE_HIGHLIGHT_API_DEPLOYMENT = os.environ.get(
    "ENABLE_HIGHLIGHT_API_DEPLOYMENT", "false"
).lower() in ("true", "1", "yes")
```

How can I resolve this? If you propose a fix, please make it concise.

Reviews (7): Last reviewed commit: "[FIX] Bool-parse ENABLE_HIGHLIGHT_API_DE..." | Re-trigger Greptile

Comment thread backend/api_v2/deployment_helper.py Outdated
Copy link
Copy Markdown
Contributor

@muhammad-ali-e muhammad-ali-e left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Added NITs

Comment thread backend/api_v2/deployment_helper.py Outdated
@vishnuszipstack vishnuszipstack changed the title [HOTFIX][FIX] Honor ENABLE_HIGHLIGHT_API_DEPLOYMENT env var on on-prem deployments [HOTFIX][FIX] Honor ENABLE_HIGHLIGHT_API_DEPLOYMENT env var on on-prem Apr 29, 2026
@vishnuszipstack vishnuszipstack changed the title [HOTFIX][FIX] Honor ENABLE_HIGHLIGHT_API_DEPLOYMENT env var on on-prem [HOTFIX][REFACTOR] Move highlight gate out of OSS Apr 29, 2026
@vishnuszipstack vishnuszipstack changed the title [HOTFIX][REFACTOR] Move highlight gate out of OSS [HOTFIX] Move highlight gate out of OSS Apr 29, 2026
@vishnuszipstack vishnuszipstack force-pushed the fix/onprem-highlight-api-deployment branch from a5e2066 to 236f94b Compare April 29, 2026 07:05
@vishnuszipstack vishnuszipstack changed the title [HOTFIX] Move highlight gate out of OSS [HOTFIX] Honor ENABLE_HIGHLIGHT_API_DEPLOYMENT env var on on-prem Apr 29, 2026
@vishnuszipstack vishnuszipstack force-pushed the fix/onprem-highlight-api-deployment branch from 236f94b to e64bc34 Compare April 29, 2026 07:17
os.environ.get returns the raw string when the variable is set, so
ENABLE_HIGHLIGHT_API_DEPLOYMENT="False" was truthy in Python (any
non-empty string is truthy). Wrap in CommonUtils.str_to_bool so
"False" / "false" / "0" actually evaluate to False.

The setting is consumed by the cloud configuration plugin's spec
default (ConfigSpec.default in plugins/configuration/cloud_config.py)
on cloud and on-prem builds. With this fix, an admin who explicitly
sets the env var to a falsy string sees highlight data stripped as
expected.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
@sonarqubecloud
Copy link
Copy Markdown

Comment thread backend/backend/settings/base.py
@vishnuszipstack vishnuszipstack merged commit 9603568 into v0.161.4-hotfix Apr 29, 2026
6 checks passed
@vishnuszipstack vishnuszipstack deleted the fix/onprem-highlight-api-deployment branch April 29, 2026 09:46
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants